DaalaReview - Revision history

MrZeus: /* Team reviews */

2015-08-18T23:16:15Z

Team reviews

← Older revision		Revision as of 23:16, 18 August 2015
Line 38:		Line 38:
	*3) Code discussed with entire team		*3) Code discussed with entire team
	*4) Suggestions for improvement (with emphasis on algo)		*4) Suggestions for improvement (with emphasis on algo)

			[[Category:Daala]]

MrZeus: /* Examples */

2014-12-25T00:16:29Z

Examples

← Older revision		Revision as of 00:16, 25 December 2014
Line 24:		Line 24:

	== Examples ==		== Examples ==
	1) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=da726b32 da726b32], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=53362b77 53362b77]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=da726b32 da726b32], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=53362b77 53362b77]
	2) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=212d13b3 212d13b3],		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=212d13b3 212d13b3],
	3) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=76ee32c8 76ee32c8], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=75b62b23 75b62b23]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=76ee32c8 76ee32c8], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=75b62b23 75b62b23]
	4) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=7c102188 7c102188], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=50732d66 50732d66]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=7c102188 7c102188], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=50732d66 50732d66]
	5) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=8cd46564 8cd46564], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=74dc3616 74dc3616]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=8cd46564 8cd46564], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=74dc3616 74dc3616]
	6) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=208ea300 208ea300]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=208ea300 208ea300]
	7) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=b4bfee65 b4bfee65]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=b4bfee65 b4bfee65]
	8) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=7cc3cc5f 7cc3cc5f], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=487fe063 487fe063]		* [https://git.xiph.org/?p=daala.git;a=commitdiff;h=7cc3cc5f 7cc3cc5f], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=487fe063 487fe063]

	== Team reviews ==		== Team reviews ==

Bbrittain at 18:37, 19 June 2013

2013-06-19T18:37:11Z

← Older revision		Revision as of 18:37, 19 June 2013
Line 11:		Line 11:
	*1) Generally no review on comments/whitespace/style/documentation		*1) Generally no review on comments/whitespace/style/documentation
	** Except potentially when changing Doxygen API comments		** Except potentially when changing Doxygen API comments
	*2)Post-review on code that is already badly broken, or on simple bug fixes		*2) Post-review on code that is already badly broken, or on simple bug fixes
	*3) No review on initial commit of new stuff that isn't being called		*3) No review on initial commit of new stuff that isn't being called
	** Full-review required to hook any code not reviewed because of point 3		** Full-review required to hook any code not reviewed because of point 3

Bbrittain: fixed ) abiguity

2013-06-19T18:33:33Z

fixed ) abiguity

← Older revision		Revision as of 18:33, 19 June 2013
Line 13:		Line 13:
	*2)Post-review on code that is already badly broken, or on simple bug fixes		*2)Post-review on code that is already badly broken, or on simple bug fixes
	*3) No review on initial commit of new stuff that isn't being called		*3) No review on initial commit of new stuff that isn't being called
	** Full-review required to hook any code not reviewed because of 3)		** Full-review required to hook any code not reviewed because of point 3
	** If code in 3) breaks the build, it gets fixed immediately or removed		** If code in point 3 breaks the build, it gets fixed immediately or removed
	** No review on new tools until they get used by other people		** No review on new tools until they get used by other people
	*4) Code that is (according to the committer) highly unlikely to break things or that is trivially testable gets post-review		*4) Code that is (according to the committer) highly unlikely to break things or that is trivially testable gets post-review
Line 20:		Line 20:
	*6) Refactoring of someone else's code gets design review, followed by either post-review (or full review if requested during design review)		*6) Refactoring of someone else's code gets design review, followed by either post-review (or full review if requested during design review)
	*7) Disruptive work (e.g. block switching) gets both design review and full review.		*7) Disruptive work (e.g. block switching) gets both design review and full review.
	*8) Most build system changes get no review (unless 0) applies)		*8) Most build system changes get no review (unless point 0 applies)
	*9) Post-review on test code		*9) Post-review on test code

Jmspeex at 18:30, 19 June 2013

2013-06-19T18:30:07Z

← Older revision		Revision as of 18:30, 19 June 2013
Line 16:		Line 16:
	** If code in 3) breaks the build, it gets fixed immediately or removed		** If code in 3) breaks the build, it gets fixed immediately or removed
	** No review on new tools until they get used by other people		** No review on new tools until they get used by other people
	*4) Code that is ~~"obviously correct"~~ or that is trivially testable gets post-review		*4) Code that is (according to the committer) highly unlikely to break things or that is trivially testable gets post-review
	*5) Code that may have subtle implications gets full review		*5) Code that may have subtle implications gets full review
	*6) Refactoring of someone else's code gets design review, followed by either post-review (or full review if requested during design review)		*6) Refactoring of someone else's code gets design review, followed by either post-review (or full review if requested during design review)

Derf at 18:21, 19 June 2013

2013-06-19T18:21:41Z

← Older revision		Revision as of 18:21, 19 June 2013
Line 24:		Line 24:

	== Examples ==		== Examples ==
	1) da726b32, 53362b77		1) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=da726b32 da726b32], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=53362b77 53362b77]
	2) 212d13b3,		2) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=212d13b3 212d13b3],
	3) 76ee32c8, 75b62b23		3) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=76ee32c8 76ee32c8], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=75b62b23 75b62b23]
	4) 7c102188, 50732d66		4) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=7c102188 7c102188], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=50732d66 50732d66]
	5) 8cd46564, 74dc3616		5) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=8cd46564 8cd46564], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=74dc3616 74dc3616]
	6) 208ea300		6) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=208ea300 208ea300]
	7) b4bfee65		7) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=b4bfee65 b4bfee65]
	8) 7cc3cc5f, 487fe063		8) [https://git.xiph.org/?p=daala.git;a=commitdiff;h=7cc3cc5f 7cc3cc5f], [https://git.xiph.org/?p=daala.git;a=commitdiff;h=487fe063 487fe063]

	== Team reviews ==		== Team reviews ==

Jmspeex: Created page with "This is a '''proposal''' for a more flexible review process. It is a set of guidelines for the most appropriate approach for different types of changes. Above all, use your judge..."

2013-06-19T18:05:48Z

Created page with "This is a '''proposal''' for a more flexible review process. It is a set of guidelines for the most appropriate approach for different types of changes. Above all, use your judge..."

New page

This is a '''proposal''' for a more flexible review process. It is a set of guidelines for the most appropriate approach for different types of changes. Above all, use your judgement.

== Definitions ==
*A) Full review: Same as current process. Can be based on Rietveld, "git format-patch", or a pull request
*B) Post-review: Code gets committed to repo first, committer nags reviewer weekly if necessary
*C) Design review: Discuss with original author (and possibly others) for the principle of a change, can (and probably should) be done even before doing the work
*D) No review: self-explanatory

== Guidelines ==
*0) Full-review on anything the committer has reasons to believe would be controversial, buggy, ...
*1) Generally no review on comments/whitespace/style/documentation
** Except potentially when changing Doxygen API comments
*2)Post-review on code that is already badly broken, or on simple bug fixes
*3) No review on initial commit of new stuff that isn't being called
** Full-review required to hook any code not reviewed because of 3)
** If code in 3) breaks the build, it gets fixed immediately or removed
** No review on new tools until they get used by other people
*4) Code that is "obviously correct" or that is trivially testable gets post-review
*5) Code that may have subtle implications gets full review
*6) Refactoring of someone else's code gets design review, followed by either post-review (or full review if requested during design review)
*7) Disruptive work (e.g. block switching) gets both design review and full review.
*8) Most build system changes get no review (unless 0) applies)
*9) Post-review on test code

== Examples ==
1) da726b32, 53362b77
2) 212d13b3,
3) 76ee32c8, 75b62b23
4) 7c102188, 50732d66
5) 8cd46564, 74dc3616
6) 208ea300
7) b4bfee65
8) 7cc3cc5f, 487fe063

== Team reviews ==
*1) Once in a while, we should pick a piece of code
*2) Original author cleans up code, adds comments
*3) Code discussed with entire team
*4) Suggestions for improvement (with emphasis on algo)