-
-
Notifications
You must be signed in to change notification settings - Fork 328
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 keeping releases drafts via repo-level GHA variable #1499
Conversation
That's great, thank you! I have set the variable in this repository to And as always - what helps you does help me as well. Do feel free to add more as you discover new needs for it, or make other changes that make your work easier. |
This is not related to releasing, but I would find it useful to run CI in feature branches in my fork without having to modify the workflow (i.e., using another branch with additional changes to make CI run) or open a PR. I'm not sure if this should be done, though, since running it on both There may be ways to avoid that, but I would be concerned about the complexity of dong so. It might be achievable by introducing a new job that other test jobs declare a dependency on, that checks if the workflow is running on a push trigger from the same repository where the branch has an open PR. I'm not sure what the commit in the upstream repository would show for its CI status, though. The opposite approach of cancelling pull_request triggered jobs when there is a corresponding push triggered job might work better in most cases, because it would prevent a duplicate job even for the commit that a PR is opened with. But unless it is only acting for jobs that run at the same time, it would have the problem that when a PR is opened much later such that changing conditions, e.g. with dependencies, would cause something to fail, this would not be discovered. That caveat is also a reason not to take the approach of removing the pull_request trigger and leaving only the push trigger, though the bigger reason to avoid that is that contributions who open PRs may not have enabled CI in their forks. Maybe the best thing to do is to run CI on the push trigger on main as is currently done and also on feature branches that are named according to a particular convention. This would lead to feature branch names that would be longer or less intuitive, but it should be straightforward to do, and it would not require any logic for checking what other runs exist, cancelling anything, or adding any new jobs. |
I suppose this would affect me as I currently push to branches in this repository that also have a PR usually, but I am more than happy to push into a fork instead. That way, this repository could be keep cleaner, I suppose many branches have accumulated by now.
That would also work, but like I said, I'd think it would be good to have an incentive not to push branches directly into this repository. If you were to submit a PR for this, I'd start working with a fork - it will be very welcome. |
How would you work with a fork? As I understand it, personal accounts on GitHub can't contain forks of their own repositories. Would you create an organization and have it fork the repository, then contribute from there? Or the reverse, creating an organization and moving this repository there, then creating the fork in your personal account (which would then have to have a name other than Would you have CI turned off in your fork? If so, then you would be unable to attain the benefits that I seek in trying to enable the push trigger for feature branches. But if the workflow would be enabled in your fork, then I am not sure the change that we are discussing would actually provide an incentive not to push feature branches directly to this upstream repository. This would depend on how rate-limits are shared between repositories. If a user creates an organization and runs CI both in an org-owned repository and in a personally owned repository, are those totally separate and independent caps? If not, then the push trigger in the fork and the pull_request trigger in the upstream repository would be slowing each other down (in the sense of making each other wait for available runners) in much the same way as when it is all done here. Anyway, I would be happy to expand the push trigger to all branches, if that is something you want done. But some of the above may be reasons not to do so, or not to do so at this time, and may also touch on other changes that would have to be made (for example, if moving this upstream repository into a newly created GitHub organization, even though the original URL should work as a redirect, it should nonetheless be updated in a number of places). So I figured I would bring that stuff up before proceeding. |
🤦♂️ With that clarified, maybe it's plan B then with glob that runs CI for feature branches/branches with a certain pattern. Maybe That shouldn't trigger for any of my branches then. Is this something you have been proposing, or will this be another 🤦♂️ moment for me 😁? |
Yes, this would be a case of the "named according to a particular convention" alternative (and the end of #1499 (comment)). I'll see if I can think of a name like Assuming that name is used, then the patterns would probably be |
I have another thought: Would some of the considerations that had motivated you to want to contribute via a fork be mitigated by giving your branches distinctive names, such as with prefix like I only advocate this if that is already valuable in its own right. But if so, and if those are the ones that should not run on a push trigger, then perhaps, if it can be managed with the available syntax, it would be best to run CI checks for the push event on all branches except those. The reason is that CI is turned off automatically in forks. Various occurrences relating to the introduction of new workflows enable it, but it is mainly enabled by deliberately doing so in the Actions tab. So any fork with CI turned on is probably a fork whose owner wants CI checks to run even separate from pull requests. What do you think of this? An alternative could be to do the opposite: any branch with |
A valid point! I find myself creating single-word branches a lot though, but can probably train myself to use a prefix to exclude it from CI. But then I once again think it should be self-describing, like Feel free to submit a PR, let's just try it - mistakes I will definitely make 😁. |
I think this is good for troubleshooting whether and why CI ran, but actually bad otherwise, because it is likely to be confused with the content or purpose of the branch (i.e., with how one would ordinarily name the branch). However, while I think we disagree about the desirability of the branch names being explicit:
So self-describing is the way to go.
I've opened #1507, which seems to me may be consistent with both our goals and preferences.
Naming is tough because, in my view, the primary purposes of a branch name is to distinguish the branch from other branches and to describe the content of the commits on the branch, and the primary purpose of a directory-like prefix is to group branches by what they are intended to do, rather than by how they are intended to be tested. Nonetheless, a In the specific case of To an extent this kind of thing could happen with any explicit naming convention, but in #1507 I went with |
With this, the "Publish the release" step has an
if:
that will be true unless a GitHub Actions configuration variable namedDRY_RUN_RELEASE
--that is, aDRY_RUN_RELEASE
in thevars
context, not to be confused with an environment variable--is defined with one of the value that is often used in GHA workflows to express true conditions. The way this works is:true
,yes
, and1
is treated as being falsy. (That is with respect to the value. Variable names themselves are automatically upcased when set.) This very deliberately includes the empty string, as obtained when the variable does not exist.It seems to me that the added complexity is worthwhile to avoid having to add
if: false
while developing and experiment with the release workflow, to suppress the publication of a release, i.e., setting the draft release to non-draft:if:
in commits that are conceptually about something else (or having additional commits just to do it). That is, I see the main benefit as making the history of the file easier to understand in the future. The convenience is also a benefit, though.The added complexity is only in the workflow itself. No burden is imposed on actual releasing, or on a repository whose maintainer elects not to add the variable:
I anticipate that, if you choose to merge this, then you would probably not set the variable here, and that, if you do, then you would probably set it to a value like
false
. However, this does not look at whether the repository is a fork, and its use is not limited to forks: if you merge this and want to test changes to the release workflow here, you can set the variable to one of the three recognized truthy values to keep the workflow from marking releases non-draft (which would affect all runs that start after the change).Here's a workflow run that tests this, and here's the affected job.