Ignore unmodified files when using --new-pr or --update_pr#4753
Open
Flamefire wants to merge 8 commits intoeasybuilders:developfrom
Open
Ignore unmodified files when using --new-pr or --update_pr#4753Flamefire wants to merge 8 commits intoeasybuilders:developfrom
--new-pr or --update_pr#4753Flamefire wants to merge 8 commits intoeasybuilders:developfrom
Conversation
Flamefire
force-pushed
the
better-new-pr
branch
from
ae86d2e to
a97229b
Compare
boegel
modified the milestones:
next release (5.0.1 or 5.1.0),
release after next release (5.0.1 or 5.1.0)
Flamefire
force-pushed
the
better-new-pr
branch
2 times, most recently
from
eeb9ded to
0ef5fd2
Compare
Contributor
Author
|
Rebased |
Member
|
@Flamefire Another merge conflict to fix... |
When including an unmodified EasyConfig with `--new-pr` an error is shown that a commit message is required because this EC is modified which is not the case. Adjust the `copy_*` functions to ignore any unmodified file. This especially ommits them in the `file_info` struct returned that is then used to determine commit message/title etc. Fixes easybuilders#4751
Flamefire
force-pushed
the
better-new-pr
branch
from
0ef5fd2 to
8d8e711
Compare
Contributor
Author
|
Rebased only |
When all passed easyconfigs got filtered out creating the branch label fails with an IndexError. Catch this case early.
Avoid calling `git fetch` muliple times by mocking that call and copy the existing remote directory.
Flamefire
force-pushed
the
better-new-pr
branch
from
78f4bc3 to
4f52eac
Compare
Contributor
Author
|
I noticed that when you pass only unmodified Easyconfigs it will crash when trying to determine a label for the branch. This can only be caught in a test actually opening a new PR (in dry-run) so I extended the new_pr/update_pr test. |
Contributor
Author
|
Ok, that seems to work now. I'd say we should get this in soon to be able to catch any remaining issues/corner cases I didn't think of. @boegel ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When including an unmodified EasyConfig with
--new-pran error is shown that a commit message is required because this EC is modified which is not the case.Adjust the
copy_*functions to ignore any unmodified file. This especially omits them in thefile_infostruct returned that is then used to determine commit message/title etc.Fixes #4751
Note: While working on this I noticed that new EasyConfigs that are dependencies of added EasyConfigs are ignored in the commit message. I propose to populate
file_infoinstead of thedep_infoateasybuild-framework/easybuild/tools/github.py
Line 1071 in b37f707
This would also fix the possibly incomplete debug log message
Staging all <n> new/modified easyconfigsthat doesn't account for those ECs.Note: View without whitespace changes due to changed indent.
I noticed that when you pass only unmodified Easyconfigs it will crash when trying to determine a label for the branch. This can only be caught in a test actually opening a new PR (in dry-run) so I extended the new_pr/update_pr test.
This increased the time of this (single) test from 58s to 72s.
As most of the time is spent in
git fetchI did some mocking to bring the time down to 38s