Make WordPress Core

Opened 8 months ago

Last modified 8 months ago

#57556 new enhancement

Configure feature branches on GitHub to trigger workflows to support development on forks

Reported by: flixos90's profile flixos90 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Build/Test Tools Keywords: has-patch needs-docs
Focuses: Cc:

Description

When contributing to WordPress core through GitHub, the workflow is to create new branch in a fork, and open a pull request from there to WordPress core.

This works reasonably well for smaller pieces of work, but when it comes to larger features it is not a great development experience having to work in a single pull request for the entire effort. A simplified workflow can be to work in individual issues and pull requests within the fork, until a fully functional version of the feature or larger enhancement is ready for an actual WordPress core pull request.

This workflow is already possible today, however one caveat is that as long as the development happens on the fork, the GitHub workflows are not triggered unless the work happens against the trunk branch. That however is not a good idea in itself as the trunk branch is the main development branch and not intended for work on specific features. It would make it impossible to work on multiple features in the same fork if they had to be developed against trunk.

This ticket therefore proposes to add a certain branch pattern to the CI configuration of WordPress core's GitHub workflows, e.g. to ensure that they run for any branches satisfying the feature/* pattern.

Of course in theory any fork could implement that individually, but that would eventually not be a great workflow as then every PR against the main WordPress/wordpress-develop repository would include that change, which it should not.

Since no actual development happens within the WordPress/wordpress-develop repository directly, I'd argue there is no harm in adding this extra branch rule. What it would help though is to support the above workflow to develop greater features iteratively in a fork.

Change History (8)

This ticket was mentioned in PR #3908 on WordPress/wordpress-develop by @flixos90.


8 months ago
#1

  • Keywords has-patch added

#2 @joemcgill
8 months ago

I think this is a great suggestion and would greatly help contributors review smaller changes as part of larger feature projects.

#3 @peterwilsoncc
8 months ago

Preventing actions from running on forks was an intentional decision in [49781].

Allowing actions to run on forks was using minutes on contributors accounts, and potentially costing them money.

An exception exists for pull requests against the forks, so in the use case you describe a pull request can be opened against the fork and the action will run if actions are enabled on the repository.

See this pull request against my fork to demonstrate running actions.

#4 @flixos90
8 months ago

@peterwilsoncc Thank you for the additional context, that's really helpful.

I believe the exception you're referring to only works when opening a fork PR against trunk though. My proposal here would be to expand that to allow for the same to happen when working against feature/* branches. As outlined in the ticket description, working against trunk in a fork is not really feasible when working on a bigger feature that consists of several PRs. It's more appropriate to work against feature specific branches. Also when working against trunk it's not possible to work on multiple decoupled features within the same fork.

#5 @jorbin
8 months ago

  • Keywords needs-docs added

I think the reasons that @peterwilsoncc mentions are valid, but I think we can get around them with some documentation and making it specific to a naming convention. I worry that feature/* is too generic though. Maybe we make it something WP specific? wp-pipeline/* perhaps to indicate that this is change that should trigger WordPress pipeline actions?

Adding the needs docs keyword since this is something that we will want to communicate in both a short-term sense on make/core but also in a long-term sense by making either a handbook update or perhaps also a CONTRIBUTING.md update.

#6 @peterwilsoncc
8 months ago

It it's possible to verify intent, then I am happy to allow them to run.

An alternative I was thinking of was to require the branch include a file .run-gh-pipelines-and-accept-charges-against-fork -- if such a thing is possible with the action config files.

#7 follow-up: @flixos90
8 months ago

@jorbin @peterwilsoncc Given the additional context you provided, I agree we would want to verify intent to run the CI workflows. I think a branch name prefix like wp-pipeline/* works well for that purpose. I think including a specific file does not work so well as it would need to be created per branch, and then it would also inevitably show up in the eventual PRs against WordPress/wordpress-develop - unless it was deleted, but then also CI workflows would stop running for PRs in the fork. So I think a specific branch name prefix works better.

Adding the needs docs keyword since this is something that we will want to communicate in both a short-term sense on make/core but also in a long-term sense by making either a handbook update or perhaps also a CONTRIBUTING.md update.

I am happy to draft some documentation for that. Related question though, is there any documentation on how this currently works? Where is it documented that fork PRs against trunk trigger GitHub workflows for example?

#8 in reply to: ↑ 7 @peterwilsoncc
8 months ago

Replying to flixos90:

I am happy to draft some documentation for that. Related question though, is there any documentation on how this currently works? Where is it documented that fork PRs against trunk trigger GitHub workflows for example?

I'm not sure that it is. I only realised when I was hunting down the historical commit and found it documented there.

You're right, a branch prefix makes more sense than a requirement. A lot less error prone. :)

Note: See TracTickets for help on using tickets.