Skip to content
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

Fix library update for git #695

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

rpatel3001
Copy link

Fetch instead of pull so tags work in sync-version, and do it first so we get updates before checking out the version.

@rpatel3001
Copy link
Author

Needs some work, this doesn't work for branches anymore

@olofk
Copy link
Owner

olofk commented Jun 25, 2024

Just let me know when you have something I should review. I remember that there were some tricks done to correctly get tags and branches.

@rpatel3001
Copy link
Author

rpatel3001 commented Jul 14, 2024

not actively working on this but pasting a link for later
https://stackoverflow.com/a/18222877/2708313

@rpatel3001
Copy link
Author

rpatel3001 commented Dec 15, 2024

i don't know that anything fancy is really needed here, I reverted my previous change and just had it not print out the scary red error message if git returns non-zero. It still prints out the git output

You are not currently on a branch.
Please specify which branch you want to merge with.
See git-pull(1) for details.

    git pull <remote> <branch>

when sync-version is a tag or hash because the Launcher class is a pretty thin wrapper and you can't control that, but if a library is on a tag or commit maybe a user should just expect that. a library update for tags or commits should probably just be no-ops. It might be desirable to have Launcher return the process output and re-raise the RuntimeError if that text isn't in the output, just in case there are other reasons git fails and you want the scary red message back?

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