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

Reconsider including vendored scripts in ingest template #12

Open
joverlee521 opened this issue Oct 14, 2023 · 2 comments
Open

Reconsider including vendored scripts in ingest template #12

joverlee521 opened this issue Oct 14, 2023 · 2 comments

Comments

@joverlee521
Copy link
Contributor

This didn't occur to me until I was reviewing the nextstrain/dengue#13 and saw the vendored scripts were pulled in separately. This is nice because then the vendored/.gitrepo file is set up correctly from the start. The seasonal-cov repo was copied from this template, so the vendored/.gitrepo file points to a parent commit in this repo.

This means when you try to pull in the latest vendored scripts from within seasonal-cov, you run into an error:

$ cd seasonal-cov
$ git subrepo pull ingest/vendored
git-subrepo: Command failed: 'git rev-list --reverse --ancestry-path --topo-order 143581871f1fdcc3ce0d020e1c3ddddffef2ffa9..HEAD'.

I had two main reasons for including the vendored scripts in the template.

  1. I didn't want to require users to set up the subrepo
  2. I wanted to make sure that the use of the scripts within the template matched the version of the vendored scripts included.

If we keep the vendored scripts in the template, we just need to document the workaround for the error.
Technically the workaround is already documented in the vendored README, so we just need to point those docs for troubleshooting.

On the other hand, if we don't include the vendored scripts in the template, then we would need additional steps for setting up the ingest workflow:

  1. User needs to install git-subrepo
  2. User needs to set up the subrepo from a specific commit hash with:
git subrepo clone https://github.com/nextstrain/ingest ingest/vendored -b <commit_hash> 

If they ever want to update the subrepo to the latest version in the future, they would also need to specify the branch in the pull:

git subrepo pull ingest/vendored -b main
@joverlee521
Copy link
Contributor Author

I've convinced myself that we should keep the vendored scripts in the template after writing everything out 😄

@joverlee521
Copy link
Contributor Author

Reopening this to remove the vendored scripts from ingest once we move all the potential augur curate scripts to official Augur commands. This will reduce burden on external users to install and manage the subrepo.

Prompted by discussion around the future ingest tutorials.

@joverlee521 joverlee521 reopened this Jan 19, 2024
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

No branches or pull requests

1 participant