Skip to content

Sane doc reports #7641

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 22, 2020
Merged

Sane doc reports #7641

merged 4 commits into from
Jun 22, 2020

Conversation

agamm
Copy link
Contributor

@agamm agamm commented Jun 17, 2020

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: #1836

Description

  • Updates the sane-doc-reports docker
  • Adds logo arguments

Screenshots

--

Minimum version of Demisto

  • 4.5.0
  • 5.0.0
  • 5.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

Demisto Partner?

  • The title must be in the following format: [YOUR_PARTNER_ID] short description

@agamm agamm requested a review from ronykoz June 17, 2020 17:04
@@ -1,5 +1,5 @@
## [Unreleased]

- Updating sane doc reports and adding customer logo.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the new release notes format
demisto-sdk update-release-notes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting to update release notes.
No changes were detected.

?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amshamah419 can you take a look at this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@agamm Can you try updating your demisto-sdk?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ronykoz @amshamah419 I updated before:
image

(Already tried: pip3 install --upgrade demisto-sdk) anything I need to change in the code / make changes ?

'utf-8')
paper_size = demisto.args().get('paperSize', 'A4').encode(
'utf-8')
orientation = demisto.args().get('orientation', 'portrait')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did you remove the encoding?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there was a need, unless you think there are times it will be affected?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh i'm not 100% sure, let's add this to be on the safe side

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a convention in content?
I remember that it works ok in pdf reports:
image
(at least most of them).
Maybe @glicht do you have an idea why we need to encode utf8?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the docker image is python3 we don't need it

@agamm
Copy link
Contributor Author

agamm commented Jun 21, 2020

Thanks @amshamah419 !

@agamm agamm merged commit 02baf24 into master Jun 22, 2020
@agamm agamm deleted the sane-doc-reports branch June 22, 2020 08:10
agamm pushed a commit that referenced this pull request Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants