-
-
Notifications
You must be signed in to change notification settings - Fork 28
Add update-modules target to update them without detaching them #177
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
base: main
Are you sure you want to change the base?
Conversation
Like git submodule update --checkout but without detaching them. Reuses the existing borg.sh checkout command but resets it to the target commit without checking if the branch matches to said target commit, just like git submodule --checkout. Relates to emacscollective#176.
borg.sh
Outdated
| echo " HEAD: $head" | ||
| echo "expected: $hash" | ||
| elif [ -n "$(git status --porcelain=v1 --ignored)" ] | ||
| elif [ -n "$(git status --porcelain=v1)" ] |
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 change wasn't supposed to be here but I will include while I'm using the target.
I will remove it or adjust if so desired after polishing the PR.
|
|
||
| cmd_checkout () { | ||
| while [ $# -ne 0 ] | ||
| while [ $# -gt 0 ] |
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.
Without this change the loop would never have been entered.
Was this on purpose?
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.
Probably not 😀
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.
I think the result is the same, 1, for example is both -gt 0 and -ne 0. But of course the former makes more sense. The problem was only the shift you comment on below, I believe.
borg.sh
Outdated
| -*) usage;; | ||
| *) break;; | ||
| esac | ||
| shift |
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.
Similarly if wouldn't have removed this the loop would have eaten up all the arguments which are not option parameter.
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.
I've addressed this and other issues you had brought up.
This also involves the complete removal of option handling, which makes sense given that there are no options. You add an option here, but maybe it makes more sense to use a new command, and also a global variable instead of argument passing (but only because it's a shell script, not lisp).
Or if you feel differently, restore the option handling after rebasing (and squashing). The problem I see with that is, that both cmd_checkout and checkout have to handle the option.
04957f9 to
4472580
Compare
Like git submodule update --checkout but without detaching them.
Reuses the existing borg.sh checkout command but resets it to the
target commit without checking if the branch matches to said target
commit, just like git submodule --checkout.
Relates to #176.
There are no manual changes in this PR as I'm first waiting for feedback.