Open jenstroeger opened 1 month ago
behnazh-w
commented
1 month ago Thanks for the suggestion! I completely agree that the pylint checker offers many valuable optional checks that can enhance code quality and performance.
Please go ahead and submit the PR!
jenstroeger
commented
1 month ago Please go ahead and submit the PR!
Alright, will do 👍🏼
jenstroeger
commented
1 month ago Before I continue: there’s a significant number of consider-using-assignment-expr errors which can’t be disabled because they’re issued by the Code Style checker which covers four more other checks (see its src).
Basically, two-line idioms like this https://github.com/oracle/macaron/blob/edfe06eb3f9337f86215b253f40f53ca1a0105c9/src/macaron/slsa_analyzer/analyze_context.py#L194-L195 would become a single line using an assignment expression:
if req := self.ctx_data.get(req_name):I’m happy to make the change, though it’ll take a little effort to adjust to reading the new code.
Alternatively, I can disable the Code Style checker completely as a first change, and we can discuss this one again later.
What are the team’s thoughts on this?
behnazh-w
commented
1 month ago I’m happy to make the change, though it’ll take a little effort to adjust to reading the new code.
Alternatively, I can disable the Code Style checker completely as a first change, and we can discuss this one again later.
What are the team’s thoughts on this?
To prevent a large change set, I recommend disabling the Code Style checker in the initial PR.
The pylint checker provides a bunch of useful, optional checkers that can improve code quality and performance. I recommend enabling most (not all) of them 🤓 [^1]
Happy to provide the PR, if that helps?
[^1]: The perflint plugin is interesting, but also a tad noisy, so perhaps not the right time to integrate permanently.