Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[RFR] Remove except-pass as a required check in pylint-odoo config #115

Closed
wants to merge 1 commit into from
Closed

[RFR] Remove except-pass as a required check in pylint-odoo config #115

wants to merge 1 commit into from

Conversation

StefanRijnhart
Copy link
Member

legalsylvain
legalsylvain approved these changes Feb 1, 2022
View reviewed changes
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Thanks.

@StefanRijnhart
Copy link
Member Author

With @yajo offering the alterative of using contextlib.suppress on the pylint-odoo ticket, do we want to proceed with this? I can add a line about this to https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#39exceptions.

@StefanRijnhart StefanRijnhart marked this pull request as draft February 2, 2022 09:26
@yajo
Copy link
Member

yajo commented Feb 2, 2022

Seems a better fix IMHO.

Actually those docs say: The pass into block except is not a good practice! but there's no explanation whatsoever, so in any case they can be improved.

@OCA-git-bot
Copy link

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@legalsylvain
Copy link
Contributor

@StefanRijnhart is it this PR really in draft state ?

@legalsylvain legalsylvain mentioned this pull request Mar 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbidoul sbidoul sbidoul approved these changes

@legalsylvain legalsylvain legalsylvain approved these changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
approved ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@StefanRijnhart @yajo @OCA-git-bot @legalsylvain @sbidoul @pedrobaeza