-
Notifications
You must be signed in to change notification settings - Fork 4
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
CI issue fixing + tests #425
Conversation
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.
Looks good to me 🙂
One thing I'd say is that, if multiple things are going to be changed/fixed, could everything that is being changed/fixed be accounted for?
The bits of code I've commented on, I would just like some note in the PR or something that explains the reason for the change, because otherwise some of them look kinda random (like the change in parametrisation values for the paganin memory hook test, or the pipeline stats value assertion changes - is this fixing #433 maybe?).
Fixes #427
Fixes #431
Fixes #433
Checklist