From 52fb85df60b7be744420852db5d20fd3248fe51e Mon Sep 17 00:00:00 2001 From: Otto Liljalaakso Date: Jul 04 2024 19:07:56 +0000 Subject: Add Pull Request Guide Pull requests are an important aspect of how Fedora packaging works, and, crucially, the easiest way for new contributors to get started. Document a way how they can be used. Fixes #66 --- diff --git a/modules/ROOT/nav.adoc b/modules/ROOT/nav.adoc index c608d8a..9c4306a 100644 --- a/modules/ROOT/nav.adoc +++ b/modules/ROOT/nav.adoc @@ -27,6 +27,8 @@ * xref:Package_Update_Guide.adoc[Package Update Guide] +* xref:Pull_Request_Guide.adoc[Pull Request Guide] + * xref:Reporting_Issues.adoc[Reporting Issues] * xref:Staying_Close_to_Upstream_Projects.adoc[Staying Close to Upstream Projects] diff --git a/modules/ROOT/pages/Installing_Packager_Tools.adoc b/modules/ROOT/pages/Installing_Packager_Tools.adoc index b377ef2..dffb9c4 100644 --- a/modules/ROOT/pages/Installing_Packager_Tools.adoc +++ b/modules/ROOT/pages/Installing_Packager_Tools.adoc @@ -1,5 +1,6 @@ = Installing Packager Tools +[#installing] == Installing The `fedora-packager` package provides tools to help you setup and work with Fedora. @@ -22,6 +23,7 @@ to be able to make changes to any package, including your own. See wiki page https://fedoraproject.org/wiki/Cryptography#OpenSSH[Cryptography: OpenSSH] for more information. +[#mock] === Mock To be able to test the build procedure in a clean chroot with diff --git a/modules/ROOT/pages/Joining_the_Package_Maintainers.adoc b/modules/ROOT/pages/Joining_the_Package_Maintainers.adoc index 843dd84..0126a49 100644 --- a/modules/ROOT/pages/Joining_the_Package_Maintainers.adoc +++ b/modules/ROOT/pages/Joining_the_Package_Maintainers.adoc @@ -205,7 +205,4 @@ or on https://libera.chat/[Libera]. == One-off contributions Changes to https://src.fedoraproject.org/browse/projects/[existing packages] -can be suggested by submitting https://docs.pagure.org/pagure/usage/pull_requests.html[pull requests]. -You must have a xref:create_a_fedora_account[Fedora account] to create a pull request. -See xref:Package_Maintenance_Guide.adoc#using_fedpkg_anonymously[using fedpkg anonymously] -for instructions. +can be suggested as explained in xref:Pull_Request_Guide.adoc[Pull Request Guide]. diff --git a/modules/ROOT/pages/Package_Maintenance_Guide.adoc b/modules/ROOT/pages/Package_Maintenance_Guide.adoc index 067e543..c248763 100644 --- a/modules/ROOT/pages/Package_Maintenance_Guide.adoc +++ b/modules/ROOT/pages/Package_Maintenance_Guide.adoc @@ -435,41 +435,7 @@ who simply want to examine a package and make changes for their own use. If the intention is to submit changes to a Fedora developer, -the package repository can be forked -and a pull request submitted. -Forking is done in the https://src.fedoraproject.org[src.fedoraproject.org] web ui. -You need a https://accounts.fedoraproject.org/[Fedora account] to be able to fork. -After the fork has been created, -typical `fedpkg` session might look like this: - -.... -fedpkg clone --anonymous forks/my-username/rpms/somepackage -cd somepackage -# change the required things in the specfile. -# 'rpmdev-bumpspec' is useful for simple version updates. -gedit somepackage.spec -# download the referenced sources to local machine -spectool -g somepackage.spec -# check that the changes you made are correct -fedpkg mockbuild -# Update sources file and .gitignore if sources have changed. -# The --offline flag skips upload to the lookaside cache. -# That is left for the maintainer to do after the pull request has been merged. -# Multiple source files can be specified as required. -fedpkg new-sources --offline somepackage-0.0.2.tar.gz -fedpkg diff -fedpkg lint -# create a branch to use in the pull request -git checkout -b my-branch -git status -git add . -git commit -git push -u origin my-branch -.... - -Afterwards, -a pull request from `my-branch` to the main package repository -can be created in the _src.fedoraproject.org_ web ui. +see xref:Pull_Request_Guide.adoc#review[Pull Request Guide]. [#tips_and_tricks] == Tips and tricks diff --git a/modules/ROOT/pages/Pull_Request_Guide.adoc b/modules/ROOT/pages/Pull_Request_Guide.adoc new file mode 100644 index 0000000..23d95cf --- /dev/null +++ b/modules/ROOT/pages/Pull_Request_Guide.adoc @@ -0,0 +1,206 @@ +include::{partialsdir}/attributes.adoc[] + += Pull Request Guide + +Fedora dist-git repositories allow pull requests, +which makes it possible for anybody to +contribute to any package without having maintainer status. +Package maintainers may also choose to use pull requests +to allow co-maintainers to review the proposed changes, +and hook into dist-git <>. + +The same packaging tools and workflows that maintainers use +can be used for pull request contributions, +with the exception of certain privileged operations +that can only be performed package maintainers, +such as submitting a Bodhi update. + +This guide describes one possible workflow for submitting a pull request. +See xref:Package_Maintenance_Guide.adoc[Package Maintenance Guide] +for more comprehensive tooling guide. +It contains many options that can be used to create other workflows. + +This guide assumes that you are creating a pull request to a package named `some-package`. +The target branch for the pull request is assumed to be _rawhide_, +as Rawhide is much more common change target than release branches. +Any other release, such as `f{MAJOROSVER}`, would mostly differ in branch name. + +This guide also assumes you are not a member of the _packager_ group, +with separate notes in places where packagers should do something differently. + +== When to make a pull request? + +In general, the reasons for making pull requests to Fedora packages are similar to +reasons for making pull requests to any open source repository. +Either you are a user of the package and want to improve it for you own use, +or you are contributing to another project or package +and need to change something to unblock your own work. +Or you find something that can be improved while inspecting the package repo, +and submit a pull request just to help the maintainers. + +A Fedora-specific reason is the xref:How_to_Get_Sponsored_into_the_Packager_Group.adoc[packager sponsoring process], +which requires a packager candidate to demonstrate their packaging skills, +which can very conveniently be done via submitting pull requests. + +A very common, and often simple, case is +that Fedora's version of a package is not the latest upstream release. +Package may also miss optional dependencies or use sub-optimal build options, +so that all features offered by upstream do not work on Fedora. + +== Prerequisites + +You need a https://accounts.fedoraproject.org[Fedora account]. +In xref:Installing_Packager_Tools.adoc[Installing Packager Tools], +you also need to follow sections +xref:Installing_Packager_Tools.adoc#installing[Installing] +and xref:Installing_Packager_Tools.adoc#mock[Configuration / Mock]. + +== Cloning the repository + +Usually, package maintainers work directly in package's dist-git repository, +to which only they have access to. +For pull requests, using a fork is necessary. +While it is possible to create a fork using src.fedoraproject.org web interface +and close it using plain Git commands, +`fepdkg fork` provides a convenient and simple CLI-based method. + +---- +fedpkg clone --anonymous some-package +cd some-package +fedpkg fork +---- + +In case `fedpkg fork` complains about missing Pagure token, +follow the instructions given in the error message. + +These commands result in a local Git repository with remotes: +_origin_ for package's official dist-git repository in fetch-only mode, +and another one with you Fedora username pointing your fork that can also be pushed to: + +---- +$ git remote --verbose +origin https://src.fedoraproject.org/rpms/some-package.git (push) +username ssh://username@pkgs.fedoraproject.org/forks/username/rpms/some-package.git (fetch) +username ssh://username@pkgs.fedoraproject.org/forks/username/rpms/some-package.git (push) +---- + +== Branching + +Just as you would for any other pull request for any other project, +create a Git branch for your changes: + +---- +git switch -c my-changes +---- + +== Make changes + +For pull requests, making local changes and testing the changes locally +is just the same for maintainers. +For example: + +---- +# change the required things in the specfile. +gedit some-package.spec +# download the referenced sources to local machine +spectool -g some-package.spec +# check that the changes you made still build +fedpkg mockbuild +# Install and test the package +---- + +== Updating sources + +Only members of the _packager_ group can update sources to lookaside cache. +If downloaded source needs to be changed, +the best a non-packager can do is to update the `sources` file with the correct checksum: + +---- +fedpkg new-sources --offline some-package-1.2.3.tar.gz +---- + +If you are in the _packager_ group, you can remove `--offline` from the command, +so that the source archive is uploaded. +Otherwise, the maintainer who eventually merges the pull request has to do that. +In that case, make sure that the merge request uniquely points to the source archive. +Usually, that is done by using a URL in specfile's _Source_ tag, +but if not, a specfile comment can be used. + +== Committing and pushing + +Create a commit from your changes, +write a sensible commit message, +and push your branch to your fork: + +---- +git add +git commit +git push username HEAD +---- + +Note that if the package uses https://fedora-infra.github.io/rpmautospec-docs/[rpmautospec], +the commit message will be parsed to create the package changelog entry. + +== Creating pull request + +When you push your changes to your fork, +you get a link for creating a pull request as output: + +---- +remote: Create a pull-request for my-changes +remote: https://src.fedoraproject.org/fork/username/rpms/some-package/diff/rawhide..my-changes +---- + +You can also create pull request for any branch in the Pagure web UI for your fork. + +The pull request form asks you to fill description, +which is auto-filled with commit changelog. +Write any pertinent information there and submit. + +If you could not populate the lookaside cache in <> step, +include that info and ask for a maintainer to do that. + +[#ci] +== Continuous integration + +Dist-git repositories are connected to CI by default. +For all pull requests, a Koji scratch build and installability test is done. +Results for these appear in the pull request's web page when they are available. +Even though failing CI does not block merging, the CI results should be green. +If CI fails, it is a good idea to fix that. + +If you need to re-run the test build in pull requests, +a comment with `[citest]` will do so. + +Package maintainers may also enable more advanced +https://fedoraproject.org/wiki/Zuul-based-ci#How_to_attach_a_Pagure_repository_on_Zuul[Zuul] CI. + +== Review + +Pull request review is just like for any other open source project. +Hopefully, a maintainer quickly responds to your pull request +and either simply merges it, asks questions or requests changes. +Answer questions, do the requested changes and push them to the same branch. + +Note that many Fedora packages are maintainer by volunteers, +and there are not hard limits for the expected response time for pull requests. +Because of this, the review may only come after days or weeks. +In case you feel that response takes too long, +you can (and should) follow the steps in +xref:fesco::Policy_for_nonresponsive_package_maintainers.adoc[Non-responsive maintainer policy]. + +== After the merge + +The pull request is complete when a maintainer has merged it. +Afterwards, the maintainer still needs to: + +* Update the lookaside cache, if you could not do it. + +* Submit a build with `fedpkg build` + (since any packager can do this for any commit in a release branch for any package, + if you are a packager, you can theoretically also do this. + But in practice it will be the maintainer who merges your pull request, + because they can start the build immediately after they merge.) + +* In case changes were done to branch other than _rawhide_, + xref:Package_Update_Guide.adoc[submit a Bodhi update]. diff --git a/modules/ROOT/pages/Utilities.adoc b/modules/ROOT/pages/Utilities.adoc index e50d1f1..9127c81 100644 --- a/modules/ROOT/pages/Utilities.adoc +++ b/modules/ROOT/pages/Utilities.adoc @@ -5,14 +5,7 @@ This page attempts to list them, but is not a complete list. == Testing new builds with pull requests and CI -To test updates and changes to packages, -package maintainers are encouraged to use the continuous integration (CI) that is available on src.fedoraproject.org through pull requests. -Each pull request will automatically run a test scratch build and report results in the pull request. - -One may need to re-run the test build in pull requests. -A comment with `[citest]` will do so. - -More advanced CI using link:https://fedoraproject.org/wiki/Zuul-based-ci#How_to_attach_a_Pagure_repository_on_Zuul[Zuul] is also available. +See xref:Pull_Request_Guide.adoc#ci[Pull Request Guide]. == Checking package update impact: fedrq