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

Add support --branch together with --all mentioned in #253 #552

Open
wants to merge 3 commits into
base: release/0.4.5
Choose a base branch
from

Conversation

hever
Copy link

@hever hever commented Nov 18, 2021

No description provided.

Copy link
Collaborator

@admorgan admorgan left a comment

Choose a reason for hiding this comment

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

Independent of the technical issues I presented inline. I am concerned with a program overriding my explicit commands. I would be ok with a check to ensure that the branch existed on all effected repos and then executed accordingly or failed with a message telling me where it was missing, but to fall back to the previously configured branch after overriding it does not feel like an ok thing to do without direct user interaction. And as this run on multiple OS with multiple versions of bash getting user feedback is extraordinarily hard.

@@ -694,6 +690,16 @@ subrepo:fetch() {
error "Can't fetch subrepo. Remote is 'none' in '$subdir/.gitrepo'."
fi

if $all_wanted && [[ -n $subrepo_branch ]]; then
o "Check if the remote branch ($subrepo_branch) exists."
OUT=true RUN git ls-remote --heads "$subrepo_remote" "$subrepo_branch"
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is already a command for this git:branch-exists see 644 for an example

o "Check if the remote branch ($subrepo_branch) exists."
OUT=true RUN git ls-remote --heads "$subrepo_remote" "$subrepo_branch"
if [[ -z $output ]]; then
SAY=false OUT=true RUN git config --file="$gitrepo" subrepo.branch
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is a function to read the gitrepo file read-gitrepo-file

@@ -694,6 +690,16 @@ subrepo:fetch() {
error "Can't fetch subrepo. Remote is 'none' in '$subdir/.gitrepo'."
fi

if $all_wanted && [[ -n $subrepo_branch ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't something similar be necessary in subrepo:push?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure. I don't think anything is missing in subrepo:push. It's works fine with --all.

@hever
Copy link
Author

hever commented Nov 25, 2021

I'll try to understand you. I'm using subrepo in different use case for me it is meaningful to do this by this way.

Every command I'm using is somehow dangerous. This is life. This one could be dangerous also. But preventing users from doing stupid things is preventing them from doing the smart ones. This command potentially do what you might not want. But it is everything in git, everything is revertible. Need for user interaction is not the way IMHO. There must be good feedback and user must know what has been done.

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.

None yet

2 participants