-
Notifications
You must be signed in to change notification settings - Fork 21
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
include gufe main and latest in ci matrix #1036
Conversation
🚨 API breaking changes detected! 🚨 |
52ecb2c
to
e28bd47
Compare
🚨 API breaking changes detected! 🚨 |
🚨 API breaking changes detected! 🚨 |
🚨 API breaking changes detected! 🚨 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1036 +/- ##
==========================================
- Coverage 94.59% 92.83% -1.77%
==========================================
Files 134 134
Lines 9940 9961 +21
==========================================
- Hits 9403 9247 -156
- Misses 537 714 +177
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
🚨 API breaking changes detected! 🚨 |
🚨 API breaking changes detected! 🚨 |
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.
Couple of things:
- If you do this, then you have to ammend the env yaml file
- This will use up all our runners when we do a merge with slow tests - i.e. it's too many runners.
See https://github.com/OpenFreeEnergy/openfe/actions/runs/12186104840/job/33993854139?pr=1036#step:9:617 Even though it's set at latest, the issue you're facing is that you've already pip installed gufe, so mamba attempts to installs, finds that you already have a working version and does nothing. |
🚨 API breaking changes detected! 🚨 |
1 similar comment
🚨 API breaking changes detected! 🚨 |
8964f28
to
f2821f6
Compare
🚨 API breaking changes detected! 🚨 |
1 similar comment
🚨 API breaking changes detected! 🚨 |
0105cc4
to
b0b3038
Compare
🚨 API breaking changes detected! 🚨 |
🚨 API breaking changes detected! 🚨 |
This reverts commit 59feef6.
🚨 API breaking changes detected! 🚨 |
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.
The main question I have is "how long are we planning to do this".
One key issue I could see is that openfe adapts to match a behaviour from gufe (see for example when we dropped the key stability tests) and then "latest" fails, but that's intentional because we've fixed things to match with upstream.
🚨 API breaking changes detected! 🚨 |
@mikemhenry I see there's api-breaking changes flagged here - where can I get more details on this? edit: I found the details (here) |
🚨 API breaking changes detected! 🚨 |
1 similar comment
🚨 API breaking changes detected! 🚨 |
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.
this looks good thanks!
e04ebe8
to
cb25ce5
Compare
🚨 API breaking changes detected! 🚨 |
CI is currently failing because of changes on gufe
main
. We should (at least for now) run CI for both gufemain
andlatest
, since we expect it to pass onlatest
but fail onmain
.