From Fedora Project Wiki
(Update outdated information about the patch submission mailing list & needed subscription due to SPAM.)
(review doc)
 
(10 intermediate revisions by 2 users not shown)
Line 1: Line 1:
= Anaconda Patch Review Process =
= Anaconda Patch Review Process =


To improve the quality of commits and reduce the number of typos introduced into the anaconda source, we are adding a new mandatory patch review process. The intent is to keep this very light-weight while still increasing visibility for changes.
To improve the quality of commits and reduce the number of typos introduced into the anaconda source, Anaconda has a mandatory patch review process. The intent is to keep this very light-weight while still increasing visibility for changes.


Note that review is no substitute for having tested your code and so you should still do that even before the submission of the code for review.
Note that review is no substitute for having tested your code and so you should still do that even before the submission of the code for review.


== How to Submit Changes for Review ==
== How to Submit Changes for Review ==
=== Preparing changes to be submitted ===


For "final" review of patches, it is best to have a local git tree to which you have committed your changes as a set of patches.  If it is a small change, one commit may be sufficient.  Larger changes should be broken into a regular patch series in which at each subsequent stage of patch application, the whole is usable and testable.
For "final" review of patches, it is best to have a local git tree to which you have committed your changes as a set of patches.  If it is a small change, one commit may be sufficient.  Larger changes should be broken into a regular patch series in which at each subsequent stage of patch application, the whole is usable and testable.


Please also make sure that there is a usable commit message along with each commit.  These should be formatted with a simple summary (~ 70 characters or less), a blank line and then a lengthier description (if necessary)
Please also make sure that there is a usable commit message along with each commit.  These should be formatted with a simple summary (~ 70 characters or less), a blank line and then a lengthier description (if necessary).
 
Once your changes are ready to be submitted and reviewed, you can either send them as patches to the anaconda-devel-list mailing list or submit them as a pull request to the Anaconda repository on Github.
 
==== Git Configuration Tips ====
 
To make all of this work easily with git, you should set up some basic config options with '''git config --global'''.  Options that are good to set are the user.name, user.email, and sendemail.smtpserver
 
==== Syntax Checking of the Code & Running the Test Suite ====
Anaconda and other related projects include the option of running Pylint and/or a built-in testsuite. It is always a good idea to run these two on your changes before submitting them for review, ideally comparing the results of run on "clean" anaconda checked out from the master branch and result when Pylint & tests on the branch with your changes.
 
=== Sending patches to the mailing list ===
 
The mailing list used for Anaconda patch review is called '''anaconda-patches@lists.fedorahosted.org''' ([https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches listinfo], [https://lists.fedorahosted.org/pipermail/anaconda-patches/ archive]) and '''you needs to be subscribed to the list or else your emails will be rejected'''. The mandatory subscription was introduced to reduce the amount of SPAM email on the list.


Once you have this, you can use  
You can use '''git format-patch''' from the command line to generate patches in a suitable format:
   git format-patch origin/master..
   git format-patch origin/master
which will spit out a series of files named 000x-foo containing your patches.  Look to ensure that these look reasonable and then you can send the patches to anaconda-devel-list for review with a command like
This will spit out a series of files named 000x-foo containing your patches.  After checking that the content of these files looks reasonable you can send the patches to anaconda-devel-list for review with a command like
   git send-email --to anaconda-patches@lists.fedorahosted.org --suppress-from \
   git send-email --to anaconda-patches@lists.fedorahosted.org --suppress-from \
   --compose <patch files here>
   --compose <patch files here>
This will let you compose an introductory mail for the patch series; if this is a single patch, you can likely send without --compose as your commit message should be enough to make it clear what you are trying to fix.
This will let you compose an introductory mail for the patch series; if this is a single patch, you can likely send without --compose as your commit message should be enough to make it clear what you are trying to fix.


== Reviewing Process ==
=== Sending a pull request ===
 
The second options fur submitting changes to the Anaconda project is by using the GitHub pull request mechanism.
 
To submit a change begin by forking the [https://github.com/rhinstaller/anaconda Anaconda] repository on GitHub. Then create a "feature" branch in your forked repository and add your changes as one or more commits in the branch. Once done, submit your "feature" branch as a pull request to the main [https://github.com/rhinstaller/anaconda Anaconda] repo.
 
== Review Process ==
 
=== The 24 hour rule ===
 
As the Anaconda contributors are widely distributed across many timezones, there is a simple rule in place to ensure all people have an adequate chance to see and review any proposed changes before they are pushed to the source code repository:
 
'''Wait at least 24 hours after proposing a change before pushing it to the source code repository.'''
 
This explains why your change might not yet be pushed even after being approved by one of the Anaconda contributors participating on the review. The contributor thinks the change is fine but makes sure that others also have a chance to review the patch.
 
''24 hours it the minimum amount'' - especially for larger changes or something which might be expected to be controversial, a longer time window to allow for discussion should be expected. At present, there are no hard limits regarding the number of reviews required, so use your best judgement.
 
=== Branches ===


Patches should be sent to '''anaconda-patches@lists.fedorahosted.org''' ([https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches listinfo], [https://lists.fedorahosted.org/pipermail/anaconda-patches/ archive]) where it is expected that they'll be critiqued.
The main upstream development branch is called ''master'' and this is where the modRana upstream development happens. The ''master'' branch is also currently used for the builds of the Fedora Rawhide installer.


'''NOTE: due to problems with spam, you need to [https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches subscribe] to the list or your emails will bounce'''
All proposed changes should first go to the master branch, if possible. From here they can be backported the applicable release branches.


Be sure to give adequate time for people to see changes, especially larger changes or something which might be expected to be controversial. We aren't at this time going to put any strong constraints on how many reviews are needed or anything of that sort, though. Use your best judgement.
==== Fedora release branches ====


== Misc Tips ==
The Fedora release branches are named using the ''f<fedora number>-branch'' scheme and are used as source of builds for the given Fedora release. For example ''f21-branch'' corresponds to Fedora 21.


=== Git Configuration Tips ===
Not only Anaconda but also other related projects, such as Blivet and Pykickstart usually have a Fedora release branch. Other projects less dependent on the low-level Fedora infrastructure, such as Initial Setup, don't have a Fedora release branch.


To make all of this work easily with git, you should set up some basic config options with '''git config --global'''.  Options that are good to set are user.name, user.email, and sendemail.smtpserver
===== Feature work vs Fixes only =====


=== Install the git-email package ===
Contributions that introduce features to any installer packages will require significantly more time and discussion before changes are accepted. Please also note that while all new features will be considered, not all may be accepted, depending on a number of factors which can arise from discussion with the development team. Feature work should always be done upstream first, in rawhide. "Feature work" includes any major behavioral changes, UI changes/redesign, or new behavior which did not exist previously.


On Fedora 18, the ''git send-email'' command is not included in the main git package; install the ''git-email'' package to get it.
Due to the volatility and QA burden that new features impose, after branching for a Fedora release occurs, it is very unlikely that any feature work will be accepted into that version of Fedora, and only bug fixes will be allowed from that point on. Any new features proposed against Fedora in Bugzilla will be deferred to rawhide. Therefore, if you want to see your changes accepted by a certain Fedora release, make sure to propose them as early as possible.


=== Syntax Checking of Code ===
==== RHEL release branches ====
TODO


Thanks to Hans's hard work, we now have a workable pychecker config again; pleas be sure to run the script and ensure that your code does not introduce any new syntax errors.
[[Category:Anaconda]]

Latest revision as of 08:40, 8 August 2018

Anaconda Patch Review Process

To improve the quality of commits and reduce the number of typos introduced into the anaconda source, Anaconda has a mandatory patch review process. The intent is to keep this very light-weight while still increasing visibility for changes.

Note that review is no substitute for having tested your code and so you should still do that even before the submission of the code for review.

How to Submit Changes for Review

Preparing changes to be submitted

For "final" review of patches, it is best to have a local git tree to which you have committed your changes as a set of patches. If it is a small change, one commit may be sufficient. Larger changes should be broken into a regular patch series in which at each subsequent stage of patch application, the whole is usable and testable.

Please also make sure that there is a usable commit message along with each commit. These should be formatted with a simple summary (~ 70 characters or less), a blank line and then a lengthier description (if necessary).

Once your changes are ready to be submitted and reviewed, you can either send them as patches to the anaconda-devel-list mailing list or submit them as a pull request to the Anaconda repository on Github.

Git Configuration Tips

To make all of this work easily with git, you should set up some basic config options with git config --global. Options that are good to set are the user.name, user.email, and sendemail.smtpserver

Syntax Checking of the Code & Running the Test Suite

Anaconda and other related projects include the option of running Pylint and/or a built-in testsuite. It is always a good idea to run these two on your changes before submitting them for review, ideally comparing the results of run on "clean" anaconda checked out from the master branch and result when Pylint & tests on the branch with your changes.

Sending patches to the mailing list

The mailing list used for Anaconda patch review is called anaconda-patches@lists.fedorahosted.org (listinfo, archive) and you needs to be subscribed to the list or else your emails will be rejected. The mandatory subscription was introduced to reduce the amount of SPAM email on the list.

You can use git format-patch from the command line to generate patches in a suitable format:

  git format-patch origin/master

This will spit out a series of files named 000x-foo containing your patches. After checking that the content of these files looks reasonable you can send the patches to anaconda-devel-list for review with a command like

  git send-email --to anaconda-patches@lists.fedorahosted.org --suppress-from \
 --compose <patch files here>

This will let you compose an introductory mail for the patch series; if this is a single patch, you can likely send without --compose as your commit message should be enough to make it clear what you are trying to fix.

Sending a pull request

The second options fur submitting changes to the Anaconda project is by using the GitHub pull request mechanism.

To submit a change begin by forking the Anaconda repository on GitHub. Then create a "feature" branch in your forked repository and add your changes as one or more commits in the branch. Once done, submit your "feature" branch as a pull request to the main Anaconda repo.

Review Process

The 24 hour rule

As the Anaconda contributors are widely distributed across many timezones, there is a simple rule in place to ensure all people have an adequate chance to see and review any proposed changes before they are pushed to the source code repository:

Wait at least 24 hours after proposing a change before pushing it to the source code repository.

This explains why your change might not yet be pushed even after being approved by one of the Anaconda contributors participating on the review. The contributor thinks the change is fine but makes sure that others also have a chance to review the patch.

24 hours it the minimum amount - especially for larger changes or something which might be expected to be controversial, a longer time window to allow for discussion should be expected. At present, there are no hard limits regarding the number of reviews required, so use your best judgement.

Branches

The main upstream development branch is called master and this is where the modRana upstream development happens. The master branch is also currently used for the builds of the Fedora Rawhide installer.

All proposed changes should first go to the master branch, if possible. From here they can be backported the applicable release branches.

Fedora release branches

The Fedora release branches are named using the f<fedora number>-branch scheme and are used as source of builds for the given Fedora release. For example f21-branch corresponds to Fedora 21.

Not only Anaconda but also other related projects, such as Blivet and Pykickstart usually have a Fedora release branch. Other projects less dependent on the low-level Fedora infrastructure, such as Initial Setup, don't have a Fedora release branch.

Feature work vs Fixes only

Contributions that introduce features to any installer packages will require significantly more time and discussion before changes are accepted. Please also note that while all new features will be considered, not all may be accepted, depending on a number of factors which can arise from discussion with the development team. Feature work should always be done upstream first, in rawhide. "Feature work" includes any major behavioral changes, UI changes/redesign, or new behavior which did not exist previously.

Due to the volatility and QA burden that new features impose, after branching for a Fedora release occurs, it is very unlikely that any feature work will be accepted into that version of Fedora, and only bug fixes will be allowed from that point on. Any new features proposed against Fedora in Bugzilla will be deferred to rawhide. Therefore, if you want to see your changes accepted by a certain Fedora release, make sure to propose them as early as possible.

RHEL release branches

TODO