Skip to content

Avoid unnecessary detached HEAD state in boot.sh and check out stable branch in migrate.sh #240

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

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

rmacklin
Copy link
Contributor

@rmacklin rmacklin commented Aug 2, 2024

This is a follow-up to #187 with the following changes:

  • Ensure the stable branch is checked out before pulling in migrate.sh

    The checkout would be a no-op if already on stable (git would print Already on 'stable' but importantly would not return a failing exit code). It would fail if the user has local changes, but if they have local changes git pull would also fail, so there's no change there.

  • Update boot.sh to avoid 'detached HEAD' state when not necessary

    Instead of checking out FETCH_HEAD, which always puts the user in 'detached HEAD' state, we will check out ${OMAKUB_REF:-stable} so that we only end up in 'detached HEAD' state when necessary (i.e. when checking out a SHA rather than a branch).

The checkout would be a no-op if already on `stable` (git would print
`Already on 'stable'` but importantly would *not* return a failing exit
code). It would fail if the user has local changes, but if they have
local changes `git pull` would also fail, so there's no change there.
This is a follow-up to 1767107 -
instead of checking out `FETCH_HEAD`, which always puts the user in
'detached HEAD' state, we will check out `${OMAKUB_REF:-stable}` so that
we only end up in 'detached HEAD' state when necessary (i.e. when
checking out a SHA rather than a branch).
@dhh dhh merged commit 5fc1bf1 into basecamp:master Aug 2, 2024
@@ -1,5 +1,6 @@
cd $OMAKUB_PATH
last_updated_at=$(git log -1 --format=%cd --date=unix)
git checkout stable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I'm not sure about is whether omakub upgrade is supposed to be usable if you've originally installed master rather than stable. If so, this line might be undesirable.

Maybe it's better to drop this line, and just rely on the fixed boot.sh...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually ,yes, on second thought I think you're right.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Apologies for not spotting the impact to the upgrade flow in the original PR. Probably warrants a mention in the release notes to instruct users to run cd $OMAKUB_PATH && git checkout stable if they encounter "You are not currently on a branch" errors when trying to upgrade.

@rmacklin rmacklin deleted the checkout-stable-branch branch August 2, 2024 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants