-
Notifications
You must be signed in to change notification settings - Fork 7
Update antlr #165
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 #165
Conversation
|
@zachcran can you take a look at the canceled test? |
9731b8d should do it @ryanmrichard |
|
@renefritze @ryanmrichard I agree that switching to |
|
Unfortunately it looks like it didn’t. Once we fix this, we may want to setup nightly runs to catch stuff like this quicker. |
I made #166 to propose and track implementing a regularly scheduled run of the CI. |
@ryanmrichard I'm not sure what you mean; is the test still being cancelled? I think I am seeing a non-cancelled run of |
Co-authored-by: Zachery Crandall <[email protected]>
Co-authored-by: Zachery Crandall <[email protected]>
Co-authored-by: Zachery Crandall <[email protected]>
|
...and we're back to the drawing board. I thought for sure setting packaging>=24.2 would have been the fix since that is the only way I could replicate the issue. After checking the logs and rerunning the action to see it in real time, the install step is going way too fast to actually be installing from scratch. Annoyingly, the install step does not appear to be providing any output at all. My local system behaves that way when pip has nothing to do. My first thought is that we should set the action up to use a virtual environment, as I point out in #170 should be done in all CI. @ryanmrichard @renefritze Any thoughts or ideas? |
In any of my projects I would just also manage the CI env and execution with uv. And commit its lockfile. In the same kind of vein, I could export a requirements file from the working lockfile and add that instead. |
Something like this: https://github.com/renefritze/CMinx/pull/4/files |
In that PR everything but the licensing check works now @zachcran I've also created a separate PR that updated and applies licenser. I've done that with the current version 0.7 via docker. The workflow uses the older 0.4 |
|
@renefritze Sorry for the delayed response! I think switching everything over to uv will take more consideration and research than we have the bandwidth for right now on our end. I think the implementations on your branch look great, but since it sounds like none of the active maintainers on the project know uv (mainly @ryanmrichard), it is probably safer and faster to stick with the basic tools to get this PR through. Then, we can be a bit more methodical about considering and migrating to other tools when it isn't entangled with broken workflows holding up a PR. Since you've already made some changes to the CI workflows here to get things working, I'll try committing a few things to fix the workflow instead of replicating a bunch of work in a separate PR. Since I need to be making commits to your fork for them to appear on this PR (afaik), please feel free to roll back anything I do. |
zachcran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the purposes of this PR, LGTM. There will need to be more workflow maintenance to follow, though. Let's see if the current tests all pass.
|
@ryanmrichard This seems to be held up by required checks that no longer exist due to changes in this PR. The Will you update the required checks, please? |
|
🚀 [bumpr] Bumped! |
|
Many thanks for getting this through @ryanmrichard @zachcran |
|
@renefritze and thank you for your patience. Like probably everyone, we're trying to do way too many things at once... |
This updates the used Antlr version to make the project usable with Python 3.13.
Some Github Action workflows needed updating to make CI pass in my fork.