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

Update antlr version to 4.11.1 #368

Merged
merged 6 commits into from
Sep 13, 2023
Merged

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Jun 27, 2023

🚀 Pull Request

Description

Helps with #313. Not sure if it closes it, as it might still be good to make the dependency optional.

I figured out what to do with a combination of the README and this helpful comment and the PR it links to. The only changes I made manually were the version number in three files. All the other changes are from running the compile.py script. I have not attempted to understand them!

The newest version of this dependency available from conda-forge is 4.13.0, but it was noted at #313 (comment) that lots of other packages use 4.11.1, so it is likely helpful to be consistent.

Tests pass for me locally. I do not expect them to pass in CI because I have not updated the lockfiles. Can anyone tell me how to do that locally?

@rcomer
Copy link
Member Author

rcomer commented Jun 27, 2023

Huh. Tests do pass. I guess antlr4-python3-runtime is getting installed independently of the lockfiles?

@pp-mo pp-mo self-assigned this Jun 28, 2023
@rcomer rcomer force-pushed the atnlr-pin branch 2 times, most recently from 1a201e0 to f0b65fa Compare June 28, 2023 10:28
@rcomer
Copy link
Member Author

rcomer commented Jun 28, 2023

Added some notes for next time.

@rcomer
Copy link
Member Author

rcomer commented Jul 3, 2023

Codacy seems to be stuck 😕

@rcomer
Copy link
Member Author

rcomer commented Sep 7, 2023

I got a full set of green ticks! 🥳

Copy link
Collaborator

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

Taking over as @pp-mo has been deployed elsewhere for now.

Looks good to me, thanks for having a go @rcomer! There might be some fallout with dependencies but I think we can fix post-merge.

EDIT: forgot I had the 'update branch' button, so that should hopefully address the dependency thing.

@trexfeathers trexfeathers merged commit 2ac94d3 into SciTools:main Sep 13, 2023
12 checks passed
@rcomer
Copy link
Member Author

rcomer commented Sep 13, 2023

Thanks @trexfeathers 🥳

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.

3 participants