-
Notifications
You must be signed in to change notification settings - Fork 31
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 tests on Julia 1.10+ #168
Conversation
schedule: | ||
- cron: '0 0 * * 1' # runs 00:00 UTC on every Monday |
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.
Does a failure here notify anyone within the org @mortenpi?
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.
Good point. I think it will notify whoever is the author of the last commit, which might not be ideal.
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.
Actually, it looks like it's the person who changes the cron line:
Notifications for scheduled workflows are sent to the user who last modified the cron syntax in the workflow file. For more information, see "Notifications for workflow runs."
Which would be me right now, which seems fine.
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.
https://docs.github.com/en/account-and-profile/managing-subscriptions-and-notifications-on-github/setting-up-notifications/configuring-notifications#github-actions-notification-options I think it will notify the people that actively subscribe to e.g. failed runs? I don't think that is turned on by default.
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.
Ok, I'm subscribed to failures so between us it should be sufficient.
This did not remove the libgit2 dep? |
Alternative to #167. It looks like there has been a bugfix in 1.10, and the auto-generated methods when you have positional arguments with default values:
DocStringExtensions.jl/test/TestModule/M.jl
Line 7 in ec66ad4
On 1.9:
vs on 1.10:
So I would argue that the new printing is more correct actually, and so we should instead make the tests version-conditional.
It wasn't obvious to me if there's any way to support the new printing on older Julia versions -- it looks like the information about the kwargs is just missing from the method metadata. It would be good to know which PR in Julia changed this though.
Also makes the CI to run weekly on a schedule, so that we'd maybe catch nightly regressions earlier.