Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Automatic packaging and CI test updates #386
Automatic packaging and CI test updates #386
Changes from 32 commits
0419097
081a1a0
15d5845
8266f66
8edefa5
2c42dee
cf4d623
e657c14
92c825d
09d9ded
fa05558
1f87321
6ea38bb
aa2d165
ab75ea7
73662a0
0e8b718
9a3ee02
f9eb005
e522d26
77de473
f6a2d5c
44c0555
61bc3c2
a2c1f65
28dd75a
cc2eff3
b2ff50c
125fe3f
5aa66db
c19f495
68953a4
f978dcf
d70924c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Question: Why do we not run test on
push
events ? Not super familiar with all the event types in github actions, so maybe these test are triggered through amerge_request
event or something which would be redundant with thepush
eventThere 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.
Just because I wrote a more complete test workflow to run only on
push
to the protected branches, so these would be redundant.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.
nit: As per my other comment, having specific
extras_require
would have the extra benefit to avoid duplicating thisgrep
magic several timesThere 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.
True indeed
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 it mean that these tests would not run on feature branches before merging to
develop
?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.
Exactly. I figured they are a bit excessive to be ran on every update to a PR. There are still tests in the
ci.yml
that should catch everything, these tests are run just to really ensure we didn't miss anything related to a specifc OS or python version.On the rare case something fails, we can always follow up with a fix.