mirror of https://github.com/vector-im/riot-web
				
				
				
			Add code quality review policy
This adds a new review policy which encourages tests for new features. As with everything, we'll continue to tune this based on feedback.pull/16980/head
							parent
							
								
									001b15b856
								
							
						
					
					
						commit
						c2e87dde85
					
				|  | @ -58,6 +58,35 @@ When reviewing code, here are some things we look for and also things we avoid: | |||
| * Assign issues only when in progress to indicate to others what can be picked | ||||
|   up | ||||
| 
 | ||||
| ## Code Quality | ||||
| 
 | ||||
| In the past, we have occasionally written different kinds of tests for | ||||
| Element and the SDKs, but it hasn't been a consistent focus. Going forward, we'd | ||||
| like to change that. | ||||
| 
 | ||||
| * For new features, code reviewers will expect some form of automated testing to | ||||
|   be included by default | ||||
| * For bug fixes, regression tests are of course great to have, but we don't want | ||||
|   to block fixes on this, so we won't require them at this time | ||||
| 
 | ||||
| The above policy is not a strict rule, but instead it's meant to be a | ||||
| conversation between the author and reviewer. As an author, try to think about | ||||
| writing a test when making your next change. As a reviewer, try to think about | ||||
| how you might test the area of code you are reviewing. If the reviewer agrees | ||||
| it would be quite difficult to test some new feature, then it's okay for them to | ||||
| accept the change without tests for now, but we'd eventually like to be more | ||||
| strict about this further down the road. | ||||
| 
 | ||||
| If you do spot areas that are quite hard to test today, please let us know in | ||||
| #element-dev:matrix.org. We can work on improving the app architecture and | ||||
| testing helpers so that future tests are easier for everyone to write, but we | ||||
| won't know which parts are difficult unless people shout when stumbling through | ||||
| them. | ||||
| 
 | ||||
| We recognise that this testing policy will slow things down a bit, but overall | ||||
| it should encourage better long-term health of the app and give everyone more | ||||
| confidence when making changes as coverage increases over time. | ||||
| 
 | ||||
| ## Design and Product Review | ||||
| 
 | ||||
| We want to ensure that all changes to Element fit with our design and product | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue
	
	 J. Ryan Stinnett
						J. Ryan Stinnett