Development Guidelines - Revision history

Icon: Correct numbering in `Push your patch'

2024-05-03T09:03:36Z

Correct numbering in `Push your patch'

← Older revision		Revision as of 09:03, 3 May 2024
Line 119:		Line 119:
	ssh://'''<gerrit_username>'''@review.sourcearcade.org:29418/flashprog''		ssh://'''<gerrit_username>'''@review.sourcearcade.org:29418/flashprog''

	5. Push to Gerrit: ''git push origin HEAD:refs/for/master%topic=my_wonderful_patch''.		3. Push to Gerrit: ''git push origin HEAD:refs/for/master%topic=my_wonderful_patch''.

	* If using HTTPS you will be prompted for the username and password you set in the Gerrit UI.		* If using HTTPS you will be prompted for the username and password you set in the Gerrit UI.

Icon: Clarify that increased patch context is only needed for ML reviews

2024-05-03T09:02:08Z

Clarify that increased patch context is only needed for ML reviews

← Older revision		Revision as of 09:02, 3 May 2024
Line 99:		Line 99:
	the design.		the design.

	~~For~~ patches that modify convoluted tables like <tt>struct flashchip flashchips[]</tt> in flashchips.c it ~~may make~~ sense to increase the lines of '''context''' to include enough information directly in the patch for reviewers (for example to include the chip names when changing other parameters like .voltage). To do this with git use '''git format-patch -U5''' where 5 is an example for the number of lines of context you want.		If you intend to send patches to the mailing list that modify convoluted tables like <tt>struct flashchip flashchips[]</tt> in flashchips.c, it makes sense to increase the lines of '''context''' to include enough information directly in the patch for reviewers (for example to include the chip names when changing other parameters like .voltage). To do this with git use '''git format-patch -U5''' where 5 is an example for the number of lines of context you want.

	=== Set up a Gerrit Account on review.sourcearcade.org ===		=== Set up a Gerrit Account on review.sourcearcade.org ===

Icon: /* Merging to Branches */

2023-12-08T15:58:33Z

Merging to Branches

← Older revision		Revision as of 15:58, 8 December 2023
Line 212:		Line 212:
	following rules apply, some are already enforced by Gerrit:		following rules apply, some are already enforced by Gerrit:

	* Every commit has to be reviewed and needs at least one +2 ~~that was not given by the commit's author~~.		* Every commit has to be reviewed and needs at least one +2.
	* ~~Except~~, if a ~~commit is authored by more than one person, each author may~~ +2 the ~~other author's changes~~.		* A maintainer is allowed to raise a +1 to a +2 if they trust the reviewer. If not, they should have a closer look before giving a +2.
			* Before merging, a maintainer should have a final look that the patch moves things into the right direction.
	* Merging should not take place within less than 24 hours after the patch was sent to review.		* Merging should not take place within less than 24 hours after the patch was sent to review.
	* Finally, before hitting ''Submit'', one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).		* Finally, before hitting ''Submit'', one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).

Icon: "nico.h@gmx.de: Use lighter orange instead of red"

2023-12-03T17:46:41Z

"nico.h@gmx.de: Use lighter orange instead of red"

← Older revision		Revision as of 17:46, 3 December 2023
Line 141:		Line 141:
	Finally, follow the [[#Sign-off Procedure]] to add your sign-off!		Finally, follow the [[#Sign-off Procedure]] to add your sign-off!

	<span style="color:~~red~~">Signed-off-by: Your Name <your@domain></span>		<span style="color:#cb4b16">Signed-off-by: Your Name <your@domain></span>

Icon: "nico.h@gmx.de: Split Adding/reviewing flash chips out of Development Guidelines"

2023-12-03T17:46:37Z

"nico.h@gmx.de: Split Adding/reviewing flash chips out of Development Guidelines"

@@ Line 200: / Line 200: @@
 NB: New contributors may need more detailed advices and should be told about minor issues like formatting problems more precisely.
 The result of a review should either be an accepted patch or a guideline how the existing code should be changed to be eventually accepted.
+See also [Adding and Reviewing new flash chips].
 = Merging to branches =

Icon: "nico.h@gmx.de: Update =Reviews= and =Merging to Branches="

2023-12-03T17:46:37Z

"nico.h@gmx.de: Update =Reviews= and =Merging to Branches="

← Older revision		Revision as of 17:46, 3 December 2023
Line 186:		Line 186:
	= Reviews =		= Reviews =

	All patches finally have to go through Gerrit. Though, if the author prefers, the actual reviewing process can also take place on the mailing list ~~or on github~~.		All patches finally have to go through Gerrit. Though, if the author
			prefers, the actual reviewing process can also take place on the mailing
			list.

	All contributions should receive at least a preliminary review within one week of submission by some ~~flashrom~~ developer (if that doesn't happen in time, please be patient).		All contributions should receive at least a preliminary review within
	At minimum this should include a broad indication of acceptance or rejection of...		one week of submission by some flashprog developer (if that doesn't
			happen in time, please be patient). At minimum this should include a
			broad indication of acceptance or rejection of...
	* the idea/rationale/motivation,		* the idea/rationale/motivation,
	* the implementation		* the implementation
	respectively.		respectively.

	In general, reviews should focus on the architectural changes and things that affect ~~flashrom~~ as a whole.		In general, reviews should focus on the architectural changes and things
			that affect flashprog as a whole.
	This includes (but is by no means limited to) changes in APIs and types, safety, portability, extensibility, and maintainability.		This includes (but is by no means limited to) changes in APIs and types, safety, portability, extensibility, and maintainability.
	The purpose of reviews is not to create perfect patches, but to steer development in the right direction and produce consensus within the community.		The purpose of reviews is not to create perfect patches, but to steer development in the right direction and produce consensus within the community.
Line 202:		Line 207:
	See also [Adding and Reviewing new flash chips].		See also [Adding and Reviewing new flash chips].

	= Merging to ~~branches~~ =		= Merging to Branches =

	Merging to branches is limited to the ~~"flashrom developers" group on~~		Merging to branches is limited to the flashprog maintainers. The
	~~Gerrit~~. ~~This means every patch reviewed somewhere else (e.g. mailing~~		following rules apply, some are already enforced by Gerrit:
	~~list or github) must finally be pushed to Gerrit. The~~ following rules
	apply, some are already enforced by Gerrit:

	* Every commit has to be reviewed and needs at least one +2 that was not given by the commit's author.		* Every commit has to be reviewed and needs at least one +2 that was not given by the commit's author.
	* Except, if a commit is authored by more than one person, each author may +2 the other author's changes.		* Except, if a commit is authored by more than one person, each author may +2 the other author's changes.
	* Merging should not take place within less than 24 hours after the review ~~started (i.e. the first message by a reviewer on Gerrit)~~.		* Merging should not take place within less than 24 hours after the patch was sent to review.
	* Finally, before hitting ''Submit'', one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).		* Finally, before hitting ''Submit'', one is reponsible to check that all comments have been addressed, especially if there was a negative review (-1).

Icon: "nico.h@gmx.de: Update =Branches= for flashprog"

2023-12-03T17:46:36Z

"nico.h@gmx.de: Update =Branches= for flashprog"

← Older revision		Revision as of 17:46, 3 December 2023
Line 13:		Line 13:
	continues there.		continues there.

	~~===~~ ''master'' branch ~~===~~		The historical ''staging'' branch was eventually renamed to ''master''.
			Releases of flashrom were based on this branch from 1.0 to 1.3. However,
			as regressions in both functionality and maintainability became visible
			around flashrom 1.3, a "flashrom-stable" ''main'' branch was forked from
			flashrom 1.2.

	~~The historical~~ ''~~staging'' branch was finally renamed to ''master'~~'.		=== Flashprog ''main'' Branch ===
	~~As usual there is no quality promise for the state of the code on the~~
	'~~'master'' branch. Even though we will try to keep the regression~~
	~~rate as low as possible, the main purpose of the branch is to merge~~
	~~new commits and make them available to a broader audience for testing.~~

	=== Release ~~branches~~ (e.g. ''1.0.x'') ===		Our ''main'' branch is a direct descendant of flashrom-stable which was
			discontinued on ''flashrom.org''. It's where we currently focus all our
			development. The goal hasn't changed: Merge new commits and make them
			available to a broader audience for testing, whilst trying to keep the
			regression rate as low as possible.

			=== Release Branches (e.g. ''1.0.x'') ===

	Branching for a new release can happen at any point in time when a		Branching for a new release can happen at any point in time when a
	commit (branch point) on ''~~master~~'' seems to be in good shape and was		commit (branch point) on ''main'' seems to be in good shape and was
	reasonably tested after previous invasive changes. Between the branch		reasonably tested after previous invasive changes. Between the branch
	point and the release, every fix pushed for ''~~master~~'' for a problem		point and the release, every fix pushed for ''main'' for a problem
	that also persists on the release branch shall be backported. The same		that also persists on the release branch shall be backported. The same
	also applies after the release for the latest release branch and,		also applies after the release for the latest release branch and,
Line 47:		Line 53:
	and will only add backported fixes to the release.		and will only add backported fixes to the release.

	~~Unless the~~ branch point (i.e. last common commit of ''~~master~~''		The branch point (i.e. last common commit of ''main'' and a release
	and a release branch) ~~is an RC, it~~ should be tagged as		branch) should be tagged as '''''p<major>.<minor>''''' (e.g. ''p1.0''),
	'''''p<major>.<minor>''''' (e.g. ''p1.0''), to keep track where		to keep track where we are on the ''main'' branch.
	we are on the ''~~master~~'' branch.

	= Patch submission =		= Patch submission =

Icon: "nico.h@gmx.de: Update =Patch Submission="

2023-12-03T17:46:36Z

"nico.h@gmx.de: Update =Patch Submission="

Show changes

Nico@flashrom.org/: "Nico@flashrom.org/: Update git-hooks step"

2023-12-03T17:46:36Z

"Nico@flashrom.org/: Update git-hooks step"

← Older revision		Revision as of 17:46, 3 December 2023
Line 109:		Line 109:
	=== Push your patch ===		=== Push your patch ===

	1. Install ~~Change-Id hook~~: ''~~gitdir=$(git rev-parse --git-dir); scp -p -P 29418 <gerrit_username>@review.coreboot.org:hooks/commit-msg ${gitdir}/hooks/~~''		1. Install Git hooks: ''make gitconfig''

	2. Add upstream as a remote:		2. Add upstream as a remote:

Nico@flashrom.org/: "Nico@flashrom.org/: Empty MediaWiki Message"

2023-12-03T17:46:35Z

"Nico@flashrom.org/: *Empty MediaWiki Message*"

← Older revision		Revision as of 17:46, 3 December 2023
Line 73:		Line 73:
	1. Via [https://review.coreboot.org/#/q/project:flashrom gerrit on coreboot.org], i.e. ''git push origin HEAD:refs/for/master'' ([https://coreboot.org/Git see instructions])		1. Via [https://review.coreboot.org/#/q/project:flashrom gerrit on coreboot.org], i.e. ''git push origin HEAD:refs/for/master'' ([https://coreboot.org/Git see instructions])

	2. Via our [[~~Mailinglist~~\|mailing list]]. When sending a patch via the mailing list, send it in-line instead of as an attachment so that reviewers can easily comment on specific parts of it.		2. Via our [[Contact#Mailing_List\|mailing list]]. When sending a patch via the mailing list, send it in-line instead of as an attachment so that reviewers can easily comment on specific parts of it.

	3. Github users: See [https://www.flashrom.org/Development_Guidelines#Using_Github Using Github].		3. Github users: See [https://www.flashrom.org/Development_Guidelines#Using_Github Using Github].
Line 83:		Line 83:
	The '''patch reviews''' may sound harsh, but please don't get discouraged. We try to merge simple patches after one or two iterations and complicated ones as soon as possible, but we have quite high standards regarding code quality.		The '''patch reviews''' may sound harsh, but please don't get discouraged. We try to merge simple patches after one or two iterations and complicated ones as soon as possible, but we have quite high standards regarding code quality.

	If you introduce new features (not flash chips, but stuff like partial programming, support for new external programmers, voltage handling, etc) please '''discuss your plans''' on the [[~~Mailinglist~~\|mailing list]] first. That way, we can avoid duplicated work and know about how flashrom internals need to be adjusted and you avoid frustration if there is some disagreement about the design.		If you introduce new features (not flash chips, but stuff like partial programming, support for new external programmers, voltage handling, etc) please '''discuss your plans''' on the [[Contact#Mailing_List\|mailing list]] first. That way, we can avoid duplicated work and know about how flashrom internals need to be adjusted and you avoid frustration if there is some disagreement about the design.

	For patches that modify convoluted tables like <tt>struct flashchip flashchips[]</tt> in flashchips.c it may make sense to increase the lines of '''context''' to include enough information directly in the patch for reviewers (for example to include the chip names when changing other parameters like .voltage). To do this with git use '''git format-patch -U5''' where 5 is an example for the number of lines of context you want.		For patches that modify convoluted tables like <tt>struct flashchip flashchips[]</tt> in flashchips.c it may make sense to increase the lines of '''context''' to include enough information directly in the patch for reviewers (for example to include the chip names when changing other parameters like .voltage). To do this with git use '''git format-patch -U5''' where 5 is an example for the number of lines of context you want.

Development Guidelines - Revision history

Icon: Correct numbering in `Push your patch'

Icon: Clarify that increased patch context is only needed for ML reviews

Icon: /* Merging to Branches */

Icon: "nico.h@gmx.de: Use lighter orange instead of red"

Icon: "nico.h@gmx.de: Split Adding/reviewing flash chips out of Development Guidelines"

Icon: "nico.h@gmx.de: Update =Reviews= and =Merging to Branches="

Icon: "nico.h@gmx.de: Update =Branches= for flashprog"

Icon: "nico.h@gmx.de: Update =Patch Submission="

Nico@flashrom.org/: "Nico@flashrom.org/: Update git-hooks step"

Nico@flashrom.org/: "Nico@flashrom.org/: *Empty MediaWiki Message*"

Nico@flashrom.org/: "Nico@flashrom.org/: Empty MediaWiki Message"