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

Upgrade astroid to 2.9.1 #5529

Merged
merged 2 commits into from
Dec 31, 2021
Merged

Upgrade astroid to 2.9.1 #5529

merged 2 commits into from
Dec 31, 2021

Conversation

Pierre-Sassoulas
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas commented Dec 15, 2021

Type of Changes

Type
βœ“ πŸ“œ Docs

Description

Closes #1470
Closes #3499
Closes #4302
Closes #4798
Closes #5081

@Pierre-Sassoulas Pierre-Sassoulas added Bug πŸͺ² Maintenance Discussion or action around maintaining pylint or the dev workflow labels Dec 15, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Dec 15, 2021
@DanielNoord
Copy link
Collaborator

Potentially also fixes #1470?
I didn't look into it too much, but there is something about symlinking in there.

@Pierre-Sassoulas
Copy link
Member Author

#1470 (comment) seems to indicate you're right, I'm going to see if I can add a regression test.

@coveralls
Copy link

coveralls commented Dec 19, 2021

Pull Request Test Coverage Report for Build 1640750960

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.01%) to 93.706%

Totals Coverage Status
Change from base Build 1640375350: 0.01%
Covered Lines: 14338
Relevant Lines: 15301

πŸ’› - Coveralls

@Pierre-Sassoulas
Copy link
Member Author

Sadly my regression test do not fail with astroid 2.9.0. Did you manage to reproduce #1470 @DanielNoord ? I can't with pylint 2.12.2 and astroid 2.9.0 myself.

@DanielNoord
Copy link
Collaborator

I didn't look into it too much. The issue seems too have a fairly straightforward reproducible example, that doesn't crash any longer?

@Pierre-Sassoulas
Copy link
Member Author

that doesn't crash any longer?

Maybe... The issue is that my example / regression test does not crash with pylint 1.7.1 either. I guess having a symlink functional test isn't a bad thing though.

@Pierre-Sassoulas Pierre-Sassoulas marked this pull request as draft December 19, 2021 18:40
@Pierre-Sassoulas Pierre-Sassoulas marked this pull request as ready for review December 31, 2021 12:42
@Pierre-Sassoulas Pierre-Sassoulas added the Needs astroid update Needs an astroid update (probably a release too) before being mergable label Dec 31, 2021
@DanielNoord
Copy link
Collaborator

Did we manage to make this regression test work? Or does it pass even with 2.9.0?

@Pierre-Sassoulas
Copy link
Member Author

Pierre-Sassoulas commented Dec 31, 2021

It pass even with 2.9.0, the symlink issue was actually happenning when the python installation was symlinked and not the analyzed code. But I think it's still a regression test for #1470 (the example is taken from here). Also a symlink functional tests can't hurt.

@DanielNoord
Copy link
Collaborator

Ah okay, so #1470 was resolved on 2.9.0 already?

@Pierre-Sassoulas
Copy link
Member Author

Ah okay, so #1470 was resolved on 2.9.0 already?

I don't know if it's specifically in astroid 2.9.0 (or even in astroid, it could be pylint) but the issue was already fixed.

@DanielNoord
Copy link
Collaborator

Does git store symlinks? I think I used the unittest framework previously to create a symlink with os/sys. Other than that, this is good to go I think.

@Pierre-Sassoulas
Copy link
Member Author

Does git store symlinks?

I think so, it's stored as the link to another file (I used the relative link for this reason). Maybe you can confirm by checking out this branch and opening the symlinked files ?

@Pierre-Sassoulas
Copy link
Member Author

Pierre-Sassoulas commented Dec 31, 2021

Nvm I think that ../module/symlinked_module.py would create a problem in github actions if it was not a symlink and a "normal" python file. (That would be invalid python)

@Pierre-Sassoulas Pierre-Sassoulas merged commit fa0a611 into main Dec 31, 2021
@Pierre-Sassoulas Pierre-Sassoulas deleted the upgrade-astroid-2.9.1 branch December 31, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug πŸͺ² Maintenance Discussion or action around maintaining pylint or the dev workflow Needs astroid update Needs an astroid update (probably a release too) before being mergable
Projects
None yet
3 participants