Skip to content

Improper warnings on broken transform#19

Open
ewohnlich wants to merge 3 commits into
plone:masterfrom
ewohnlich:master
Open

Improper warnings on broken transform#19
ewohnlich wants to merge 3 commits into
plone:masterfrom
ewohnlich:master

Conversation

@ewohnlich

Copy link
Copy Markdown

The broken.py module is particularly noisy. For reasons, I wanted to ignore some warnings from this module but encountered two issues when trying to set the threshold for the PortalTransforms logger above WARN.

  1. It doesn't just log the error, it also prints to the console
  2. The severity of the log message does not use logging.WARNING (30) it uses a global variable named WARNING that is set to 100. That is well above the level for CRITICAL (50)!!!

This pull request removes the print message and sets the severity to logging.WARNING.

@gforcada

Copy link
Copy Markdown
Member

@ewohnlich could you clarify the CLA status? The changes LGTM, once the CLA is clear we can merge right away 😃

@gforcada

Copy link
Copy Markdown
Member

Oh and the changelog entry! 😃

@ewohnlich

Copy link
Copy Markdown
Author

Ok, I have not contributed to Plone core before so I will need to sign the license agreement. For the changelog, should I just add a line in the bug fixes section in the 3.0.0 version?

@gforcada

Copy link
Copy Markdown
Member

Yes, thatw would be enough, thanks!

@mister-roboto

Copy link
Copy Markdown

Wohnlich, Eric (IMS) your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@mister-roboto

Copy link
Copy Markdown

Wohnlich, Eric (IMS) your emails are not known to GitHub and thus it is impossible to know if you have signed the Plone Contributor Agreement, which is required to merge this pull request.

Learn about the Plone Contributor Agreement: http://docs.plone.org/develop/coredev/docs/contributors_agreement_explained.html How to add more emails to your GitHub account: https://help.github.com/articles/adding-an-email-address-to-your-github-account/

@ewohnlich

ewohnlich commented Dec 22, 2021

Copy link
Copy Markdown
Author

@mister-roboto I've set this account to display email address. I have signed the Plone contribution agreement and contributed more but I am not sure if it was with the personal gmail account set here or my account with my employer wohnlice@imsweb.com. Please let me know if I need to update something.

edit: I didn't realize this was from such an old issue. I have no idea if it's still relevant.

@jensens

jensens commented Dec 23, 2021

Copy link
Copy Markdown
Member

@ewohnlich in git log you should see the email used for the commit.

@jensens

jensens commented Dec 23, 2021

Copy link
Copy Markdown
Member

btw., it is still an valid micro fix. Yesterday I looked through our old PRs to close a whole bunch, but yours is worth to get merged.

@Rudd-O Rudd-O self-requested a review December 25, 2021 13:18

@Rudd-O Rudd-O left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

With CLA signed, this looks good to me.

@jensens

jensens commented Dec 25, 2021

Copy link
Copy Markdown
Member

In fact I would like to see a git amend with an email address connected to the email account. see https://www.git-tower.com/learn/git/faq/change-author-name-email or https://stackoverflow.com/questions/3042437/how-to-change-the-commit-author-for-one-specific-commit

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.

5 participants