-
Notifications
You must be signed in to change notification settings - Fork 30
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
deploy to gh-pages branch #32
Conversation
Co-authored-by: Kevin Gibbons <bakkot@gmail.com> Co-authored-by: Jordan Harband <ljharb@gmail.com>
.github/workflows/build.yml
Outdated
|
||
on: [push] | ||
on: pull_request |
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 should happen on push 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.
The deploy job will already do a build, no?
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.
on main, but not on other branches pushed to the repo
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.
Do we want it to run for other branches pushed to the repo which are neither PRs nor main?
Personally I tend to make a bunch of commits on a branch while iterating, and sometimes I push those so I can pick up on a different machine, but I don't want the build job to run on all of those commits. They're not ready yet; that's why they're not in a PR.
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.
Sure, and in those cases you can just ignore any transient failures, but it's very useful in proposals to have long-running branches with potential in-flight changes, and having CI run on those is also useful.
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.
Usually you'd do those as a PR, I would think? Or, of course, edit this file on your branch to list your branch.
The default across pretty much every project I've worked on is that CI runs only on the main branch, possibly some other explicitly named branches, and open PRs. (For example, node, prettier, eslint, and typescript all work this way.) For this template repository I would very much like to stick to that default unless there is some extremely strong reason to be special.
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.
As we've seen in the past, we've got very different sets of anecdata about projects we've worked on - most of the ones I've worked on run CI on every branch. It's mostly the very rare niche cases of large projects with multiple collaborators - like node, prettier, eslint, typescript, etc - that bother skipping CI on non-main branches, since on most projects, contributions are all made from forks.
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.
Well, it is also the case that people who have set up their own CI for proposals have followed my convention, not yours: proposal-temporal, proposal-class-static-block, proposal-change-array-by-copy, etc.
e655edd
to
b7cdcdb
Compare
3126f3f
to
14953c2
Compare
Per discussion.
This does not include the fancy PR previews mentioned in #30, though I do like the idea and am happy to include it as a followup.