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

Allow optional src_path argument to generate that default to URL #174

Merged
merged 3 commits into from
May 31, 2024

Conversation

sverhoeven
Copy link
Collaborator

@sverhoeven sverhoeven commented May 29, 2024

Instead of using the copier.yml from ~/.julia/** use GH url.

TODO

  • make url an argument
  • use same branch/repo for template as where generate() function is defined, To make sure in a PR that changes both generator and template are tested together.

Related issues

Fixes #146

Checklist

sverhoeven and others added 2 commits May 29, 2024 16:22
Instead of using the copier.yml from ~/.julia/** use GH url.

Fixes #146
If the argument is passed, copy from that location, otherwise
url the URL.

BREAKING CHANGE: The default API now clones from the URL, which means
that even if you clone the repo and run the generate command, it will
use the latest tagged version.
@abelsiqueira
Copy link
Owner

Zenodo failures are 50x.

@abelsiqueira
Copy link
Owner

@sverhoeven I'm not sure I understood your second bullet point. This is testing the generation of the template using wrapper (Julia) vs direct (copier) in two situations: using the source "URL#main", and using the source "dir of cloned/added package".

@abelsiqueira
Copy link
Owner

@sverhoeven would you like to review, or should I ask @suvayu?

@abelsiqueira abelsiqueira marked this pull request as ready for review May 30, 2024 11:38
@suvayu
Copy link
Collaborator

suvayu commented May 30, 2024

I'm not sure I understood your second bullet point.

In the original commit the URL was hard coded, so if you opened a PR in this repo, CI would still run on a template url that points to main, not the branch in the PR.

In the test, if you are passing a fixed url (haven't looked at your commit yet), then it should be passing the url for the PR branch for the test to be correct.

Copy link
Collaborator

@suvayu suvayu left a comment

Choose a reason for hiding this comment

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

The test needs to address the 2nd bullet.

@abelsiqueira
Copy link
Owner

Explained offline that this actually addresses the second bullet point and the PR was approved then.

@abelsiqueira abelsiqueira changed the title Use url as source Allow optional src_path argument to generate that default to URL May 31, 2024
@abelsiqueira abelsiqueira merged commit 7d051fe into main May 31, 2024
9 checks passed
@abelsiqueira abelsiqueira deleted the 146-read-only-project branch May 31, 2024 08:54
@abelsiqueira
Copy link
Owner

@allcontributors add @suvayu for review

Copy link
Contributor

@abelsiqueira

I've put up a pull request to add @suvayu! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

project is read-only
3 participants