Postpone refs matching to after setting working dir to repo dir.#668
Open
corto-pabu wants to merge 1 commit intonewren:mainfrom
Open
Postpone refs matching to after setting working dir to repo dir.#668corto-pabu wants to merge 1 commit intonewren:mainfrom
corto-pabu wants to merge 1 commit intonewren:mainfrom
Conversation
subproc.Popen() calls in bfg-ish relies on a os.chdir() call to have been made beforehand for each child process to run in the repo dir as required. get_preservation_info has a subproc.Popen() call that before this change is executed before the required os.chdir() call.
newren
requested changes
Jun 6, 2025
Owner
newren
left a comment
There was a problem hiding this comment.
Nice catch and well spotted.
The modification itself looks good, but there's a few issues with the commit:
- All commits need a Signed-off-by trailer for me to accept them, same as required by the git project.
- Please wrap lines in the commit message at 72 characters
- First lines should be of the form "area: change summary" rather than just "change summary".
- I need a real committer rather than "GitHub noreply@github.com"; that'll probably be resolved automatically by amending your commit to add your signed-off-by, though.
Taking this all together, you probably just need to run git commit --amend -s on this commit and use a commit message that looks something like:
bfg-ish: postpone refs matching until working dir has been adjusted
subproc.Popen() calls in bfg-ish rely on an os.chdir() call to have been
made beforehand so that the child process runs in the repo directory as
required. Unfortunately, get_preservation_info() has a subproc.Popen()
call before the os.chdir(), which is problematic. Move the
get_preservation_info() call after the os.chdir() call.
Signed-off-by: <FILL THIS OUT>
If you can fix that up, I'd be happy to merge it.
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
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.
subproc.Popen() calls in bfg-ish relies on a os.chdir() call to have been made beforehand for each child process to run in the repo dir as required.
get_preservation_info has a subproc.Popen() call that before this change is executed before the required os.chdir() call.