-
Notifications
You must be signed in to change notification settings - Fork 0
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
README: Update git subrepo pull
with --force
flag
#24
Conversation
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.
(non-blocking, just answering my own question of "why was there a merge conflict?")
I was hoping that merge conflicts could be avoided if commits were applied sequentially from the last time the remote was pulled, but now I realize that's not what git subrepo
is doing (it doesn't store the last commit pulled from the remote). For example, if 6e955d7 and 6f196f7 were applied in that order, there should be no merge conflict. However, it looks like the latest contents are fetched and attempted as a single commit, thus failing on any conflicts.
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.
Yeah, I don't fully understand why there was a merge conflict. I just pulled in nextstrain/rsv#40 without the --force
flag and did not run into any conflicts.
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.
Hmm, must have something to do with the parent commit in the monkeypox repo. I was able to pull in without the --force
flag after changing the parent commit in this test branch.
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.
@joverlee521 that's weird! A manual search reveals that setting parent
to anything nextstrain/mpox@f96fa06 or later prevents merge conflict.
I'd think it's related to my force-push in nextstrain/mpox#175, but the commit where it starts having conflicts is slightly off...
$ git log --oneline --graph # with my own annotations in parentheses
…
* 3037946 (WORKS) update parent
* b766498 (WORKS) Merge pull request #175: Use centralized scripts for NCBI Virus
|\
| * 871817b (WORKS) Use centralized scripts for NCBI Virus
| * afeec9c (WORKS) git subrepo pull (merge) ingest/vendored
* | f96fa06 (WORKS) Merge pull request #176: Update CI workflow triggers
|\ \
| |/
|/|
| * 00f324e (CONFLICTS) Update CI workflow triggers
|/
* fb81750 (CURRENT-CONFLICTS) Merge pull request #173: Use default config and partial overrides
|\
| * 4140aff (CONFLICTS) Set hmpxv1-focused defaults
| * 55f7569 Move common values to default config, allow partial overrides
|/
* 12ccb42 Merge pull request #170 from nextstrain/remove-augur-recursion-limit
…
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 is making me think that we don't need to use --force
as default, but maybe just document it as a work-around for merge conflicts.
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.
That sounds fine to me.
Now that we're actively using git subrepo
, looks like it's not as seamless as we initially hoped... but I think still fine to use for now.
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.
Maybe it'd be better to centralize the currently decentralized usage snippets by putting it in nextstrain/ingest and linking to it from pathogen repo READMEs. That way we only need to make these doc updates in one place.
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.
Force pushed with --force
as a documented workaround instead of the default.
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.
Maybe it'd be better to centralize the currently decentralized usage snippets by putting it in nextstrain/ingest and linking to it from pathogen repo READMEs. That way we only need to make these doc updates in one place.
Done (I think):
In walking through `git subrepo pull` with @j23414 for monkeypox, we ran into unexpected merge conflicts.¹ Reading through the git subrepo docs,² I found that we can just make a clean "reclone" of the subrepo with the `--force` flag. Instead of working through the conflict resolution, the forced reclone made sense for just getting the latest version of the ingest repo. ¹ https://gist.github.com/joverlee521/b00739c3dcbff66ca99828d43cb80a42 ² https://github.com/ingydotnet/git-subrepo#commands
750c6dc
to
fb8b17d
Compare
Point users to the centralized usage docs of the vendored ingest repo instead of maintaining a different copy of snippets that may become out of sync. Originally suggested by @victorlin in a nextstrain/ingest PR comment¹ ¹ nextstrain/ingest#24 (comment)
In walking through
git subrepo pull
with @j23414 for monkeypox, we ran into unexpected merge conflicts.¹Reading through the git subrepo docs,² I found that we can just make a clean "reclone" of the subrepo with the
--force
flag. Instead of working through the conflict resolution, the forced reclone made sense for just getting the latest version of the ingest repo.If we are operating on a "pull-only" model for vendoring the ingest repo, using the
--force
flag seems like a reasonable default.¹ https://gist.github.com/joverlee521/b00739c3dcbff66ca99828d43cb80a42
² https://github.com/ingydotnet/git-subrepo#commands
Checklist