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.

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

Fix to maintain order of package paths #9887

Merged
merged 5 commits into from
Aug 27, 2024
Merged

Fix to maintain order of package paths #9887

merged 5 commits into from
Aug 27, 2024

Conversation

akamat10
Copy link
Contributor

Type of Changes

Type
🐛 Bug fix

Description

Refs #9883

Addresses the indeterminacy identified in #9883

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil
Copy link

codecov bot commented Aug 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.80%. Comparing base (6bb335f) to head (8901361).
Report is 99 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9887   +/-   ##
=======================================
  Coverage   95.80%   95.80%           
=======================================
  Files         174      174           
  Lines       18944    18944           
=======================================
  Hits        18149    18149           
  Misses        795      795           
Files with missing lines Coverage Δ
pylint/lint/pylinter.py 96.65% <ø> (ø)

This comment has been minimized.

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

Thank you for opening the PR and thank you for the analysis. Can you add a fragment describing the fix in https://github.com/pylint-dev/pylint/tree/main/doc/whatsnew/fragments, please ?

Also I suppose you used dict.fromkeys for performance reasons compared to sorted or something else ?

@akamat10
Copy link
Contributor Author

Thank you for opening the PR and thank you for the analysis. Can you add a fragment describing the fix in https://github.com/pylint-dev/pylint/tree/main/doc/whatsnew/fragments, please ?
Will do.
Also I suppose you used dict.fromkeys for performance reasons compared to sorted or something else ?
The intent was to avoid duplication of paths as the previous code did and also have a deterministic order. After this change, whether or not we see the error I mentioned in the issue will depend on the order of file and modules specified in the command line.

akamat10 and others added 3 commits August 22, 2024 22:24

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil
for more information, see https://pre-commit.ci

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil

This comment has been minimized.

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil
Copy link
Contributor

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 8901361

@Pierre-Sassoulas Pierre-Sassoulas merged commit 8314743 into pylint-dev:main Aug 27, 2024
44 checks passed
github-actions bot pushed a commit that referenced this pull request Aug 27, 2024

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil
(cherry picked from commit 8314743)
Pierre-Sassoulas pushed a commit that referenced this pull request Aug 30, 2024

Verified

This commit was signed with the committer’s verified signature.
himdel Martin Hradil
(cherry picked from commit 8314743)

Co-authored-by: Akhil Kamat <akhil.kamat@gmail.com>
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.

None yet

2 participants