-
Notifications
You must be signed in to change notification settings - Fork 2
feat: add workflow generator #8
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
Conversation
d4266ef to
b1135d5
Compare
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 looks pretty reasonable. Did you consider using a repo template instead? Since we always have to set up a new repo I don't the repo template approach makes as much sense as it otherwise does.
Edit: I kept reading and I see how you did things makes really good sense and that the repo template approach would make less sense.
| undefined=jinja2.StrictUndefined, | ||
| ) | ||
|
|
||
| config_path = args.repository / ".github/workflows/generate.toml" |
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.
So this would be something we'd set up beforehand?
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.
I tested and I had to at least specify this in my generate.toml to get the generate subcommand to run,
private = true
environment = "foo"
permissions = {"id_token" = "some_tok"}
driver = "somedriver"
lang = {"go"=true}can we make the minimum required even simpler? driver and lang could be required, maybe private as well. But the others could maybe be made optional?
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.
oops yeah, I've only tested on one repo so I'll go test it with some other things
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 I could have it generate a basic toml under a different command too.
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.
Updated - it'll generate generate.toml if it's not found and it's nicer about defaults 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.
That sounds great.
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.
Just tested and this works nicely.
|
Yeah I didn't even consider repo templates. My model for this is conda-smithy for conda-forge |
What's Changed
For future enhancement: