Code review in Phabricator - Revision history

Vlorentz: Fix link

2021-03-31T12:44:26Z

Fix link

← Older revision		Revision as of 12:44, 31 March 2021
Line 1:		Line 1:
	#REDIRECT [[swhdocs:devel/contributing/~~code-review~~.html#~~code~~-~~review~~]]		#REDIRECT [[swhdocs:devel/contributing/phabricator.html#patch-submission]]


	[[Category:Software development]]		[[Category:Software development]]

2021-03-22T11:11:37Z

Redirect to swhdocs

← Older revision		Revision as of 11:11, 22 March 2021
Line 1:		Line 1:
	~~This page~~ [~~https~~:~~//docs.softwareheritage.org/~~devel/contributing/code-review.html#code-review ~~moved to the main developer documentation~~]		#REDIRECT [[swhdocs:devel/contributing/code-review.html#code-review]]


	[[Category:Software development]]		[[Category:Software development]]

2021-03-18T07:52:16Z

move to the main doc

2020-09-14T14:15:21Z

Added step about rebasing concurrent commits on master

← Older revision		Revision as of 14:15, 14 September 2020
Line 112:		Line 112:
	* adding new commits, and/or		* adding new commits, and/or
	* rewriting old commits with git rebase (to preserve a nice, easy to bisect history)		* rewriting old commits with git rebase (to preserve a nice, easy to bisect history)
			* pulling on master and rebasing your branch against it if meanwhile someone landed commits on master:

			<pre>
			git checkout master
			git pull
			git checkout my-shiny-feature
			git rebase master
			</pre>


	When you're ready to '''update your review request''':		When you're ready to '''update your review request''':

2020-07-29T12:37:38Z

Dependencies between diffs

← Older revision		Revision as of 12:37, 29 July 2020
Line 128:		Line 128:
	Note that you can manage diff dependencies within the same module with the following keyword in the diff description:		Note that you can manage diff dependencies within the same module with the following keyword in the diff description:

	~~```~~		<pre>
	Depends on Dxx		Depends on Dxx
	~~```~~		</pre>

	That allows to keep a logical view in your diff.		That allows to keep a logical view in your diff.

2020-07-29T12:36:58Z

About dependencies between diffs

← Older revision		Revision as of 12:36, 29 July 2020
Line 115:		Line 115:
	When you're ready to '''update your review request''':		When you're ready to '''update your review request''':
	<pre>		<pre>
	arc diff --update Dxx ~~origin/master~~		arc diff --update Dxx HEAD~
	</pre>		</pre>

Line 123:		Line 123:
	Differential only care about the code diff, and not about the commits or their order.		Differential only care about the code diff, and not about the commits or their order.
	Therefore each "update" can be a completely different series of commits, possibly rewritten from the previous submission.		Therefore each "update" can be a completely different series of commits, possibly rewritten from the previous submission.

			=== Dependencies between diffs ===

			Note that you can manage diff dependencies within the same module with the following keyword in the diff description:

			```
			Depends on Dxx
			```

			That allows to keep a logical view in your diff.
			It's not strictly necessary (because the tooling now deals with it properly) but it might help reviewers or yourself to do so.

	=== Landing your change onto master ===		=== Landing your change onto master ===

2020-07-29T12:33:00Z

Starting a new feature and submit it for review

← Older revision		Revision as of 12:33, 29 July 2020
Line 52:		Line 52:
	=== Starting a new feature and submit it for review ===		=== Starting a new feature and submit it for review ===

	Use a '''one branch per feature''' workflow, with well-separated ''logical commits'' ([https://wiki.softwareheritage.org/wiki/Git_style_guide following those conventions])		Use a '''one branch per feature''' workflow, with well-separated ''logical commits'' ([https://wiki.softwareheritage.org/wiki/Git_style_guide following those conventions]).
	Please open one diff per logical commit to keep the diff size to a minimum.		Please open one diff per logical commit to keep the diff size to a minimum.

2020-07-29T12:32:47Z

Starting a new feature and submit it for review

← Older revision		Revision as of 12:32, 29 July 2020
Line 53:		Line 53:

	Use a '''one branch per feature''' workflow, with well-separated ''logical commits'' ([https://wiki.softwareheritage.org/wiki/Git_style_guide following those conventions])		Use a '''one branch per feature''' workflow, with well-separated ''logical commits'' ([https://wiki.softwareheritage.org/wiki/Git_style_guide following those conventions])
			Please open one diff per logical commit to keep the diff size to a minimum.

	<pre>		<pre>
	git checkout -b my-shiny-feature		git checkout -b my-shiny-feature

2020-02-18T13:21:29Z

Landing your change onto master: Recommend --ff-only when merging.

← Older revision		Revision as of 13:21, 18 February 2020
Line 133:		Line 133:
	<pre>		<pre>
	git checkout master		git checkout master
	git merge my-shiny-feature		git merge --ff-only my-shiny-feature
	git push		git push
	</pre>		</pre>

			<code>--ff-only</code> is optional, and makes sure you don't unintentionally create a merge commit.

	Optionally you can then delete your local feature branch:		Optionally you can then delete your local feature branch:

2019-06-18T13:55:07Z

VCS password for pushes

← Older revision		Revision as of 13:55, 18 June 2019
Line 41:		Line 41:
	In your forge User settings page (On the top right, click on your avatar, then click ''Settings''), you need to use the ''Authentication'' > ''VCS Password'' section to set your VCS password (Direct link: <tt>hxxps://forge.softwareheritage.org/settings/user/'''<your username>'''/page/vcspassword/</tt>).		In your forge User settings page (On the top right, click on your avatar, then click ''Settings''), you need to use the ''Authentication'' > ''VCS Password'' section to set your VCS password (Direct link: <tt>hxxps://forge.softwareheritage.org/settings/user/'''<your username>'''/page/vcspassword/</tt>).

			If you still get a 403 error on push, this means you need a forge administrator to enable HTTPS pushes for the repository (which wasn't done by default in historical repositories). Please drop by on IRC and let us know!

	== Workflow ==		== Workflow ==