QA process - what is it and where does it live?

[iteles]

As the number of collaborators and dwyl open source modules grows, it stands to reason that the number of people reviewers for PRs will also grow.

There are certain processes in place for these reviews, which we need to capture and @rjmk can certainly add some 'lessons learned' to.

Seeing as these also affect contributors, should these processes live in the contributing repo?

[iteles]

• [ ] :eyes:
• [ ] Review based on priority label
• [ ] General comments in the PR vs comments which are :warning: blockers to the code being merged ⚠️
• [ ] If PR is assigned to more than one person, any of those people can review
• [ ] What to look for while reviewing, e.g. full test coverage, specific things in the code and for bug fixing that regression tests have been added #14

[rjmk]

@rjmk can certainly add some 'lessons learned'

Is this a good place for me to pop thoughts?

[iteles]

@rjmk Yes please!

[rjmk]

Sorry, I was just on holiday so haven't had a time to jot anything down yet.

I'll try and do so before next week

[iteles]

Thanks @rjmk, hope you had a fantastic holiday!

[iteles]

@jrans If you have some time before you head to pastures new (and whilst it's still fresh 🌴 😎 ), would :heart: to get some of your QA tips too. Our client has been singing your QA praises!

What I'd ultimately like to get to is a checklist of things to look out for (QA 101 if you will!) but lessons learned would be amazing if you have time 🕐 🎉

[jrans]

My notes on QA life. I've found that a smooth QA process can make the whole project much more efficient and work more enjoyable. I strongly believe QA should be done at regular intervals and rules should be stuck to and kept strict. As soon as one member of the team doesn't abide it makes it much harder for everyone else and ultimately the project fails. You are working with each other not against each other and the progress of project is always key! These aren't hard set rules and every team is different and QA process should be adjusted to and agreed upon by everyone.

• Work out time to be spent each day Ideally 1 day per developer over 2 weeks is a good amount. That means 7hr * 1/10 = 42mins a day per developer
• Communication Outline all possible communication forms between QA and developer and when it is and is not appropriate to use them.
• QA should be at regular intervals ideally once a day at a fixed time. If you are to work on other projects yourself this needs to be strict and be either at the start or end of the day. I would strongly recommend the start of the day as developers know they will get outstanding work seen to first thing and can work late if they are desperate to have something seen. The most important thing for the QA is to not have to dip in and out of QA as this means they are very unproductive in anything else they try to do.
• Alternate between pulls fairly Goes without saying really but its important to keep each members work flowing.
• System for urgent reviews Try to employ a system that helps with urgent PRs (ones that are major bug fixes or are required by everyone else). This needs to be agreed by everyone and people may want to forgo their PRS in favour of someone else. If time sensitive you might have to review out of designated hours. This should not happen frequently and you may employ a weekly strike system if this becomes habit.
• Ordering An ordering system based on numbers in title with lowest being highest priority could be a way for developers to signal which work they want first.
• No assigning on pulls that depend on others This is so important. When a reviewer merges in a PR A and another PR B depends on it the commits on B will still be visible until master is merged into it. This means the diff is not accurate and much harder for the reviewer.
• Proof Read Developers must proof read their PR on github before assigning. This should be done with all work and its very easy with githubs diffs to see any obvious mistakes. Its a complete waste of time for a QA to report that a random file has been uploaded or there is a typo.
• Nothing is personal As QA you are naturally being critical of someones hard work! This can lead to frustration by both parties and tensions. This should not be the case. Everyone should try to help one another and put themselves in the other persons shoes. The number 1 priority is the success of the project and everyone must be hardworking and honest. QAs should be kind and fair and not break a developers morale as will lead to less work being done whilst developers should listen fully to the QA and avoid making repeated mistakes and being told something 3 times. If there are disagreements they should be settled by another member of the team and addressed swiftly so nothing builds up.
• Run project regularly The QA should always try run the project frequently so they are not out of touch with the product and actually realise what the code is contributing to. Developers should record all the instructions on how to run nicely in the readme.md and updated!
• Responsibility of work monitoring As a QA i have often seen some developers being very active on particular days and not so active for several days. As the QA is the one person who regularly sees the contributions made by each team member, an open question remains as to whether they should be reporting potential burnout or someone not performing? Obviously a code reviewer wants to focus on code not humans but as a manager it could be useful to ask QA for updates. This needs to be transparent to the developer though.
• Github are making things easier Thanks github for new review process.
• Provide system for code review comments Some comments may just be suggestions and things for developer to think about next time. These shouldn't slow down a PR going in as it consumes a lot of time making things perfect and not helpful. However some things should never be allowed in to pollute code and some things may just be erroneous and have to be addressed. You may also want to ask questions before properly reviewing stuff so you have enough context. Its therefore important to have a system that means the developer knows what to prioritise when changes are requested. A ⚠️ symbol is good for highlighting the most important things, :art: could be used for stylistic but not important changes, ❓ for questions? You decide and agree with developers!
• Natural order in which code is reviewed As a reviewer I will always unless told otherwise prioritise smaller PRs. Simply as they are easier for me and much more attractive. That means bigger ones will always get left to last/have not enough time for on a given day and become more and more messy. This means a developer should always assign little and often and if reviewed regularly should not have their work slowed down.
• Encourage developers to ask before doing You as QA are likely to have more experience than developer and therefore it can be much more efficient for a developer to use their QA time by speaking to QA before they implement something if unsure rather than waste time having to completely redo something because they missed something to begin with that someone more experienced could have pointed out.
• Repeated mistakes should be called out If a developer repeatedly makes a mistake after being told many times they need to be spoken to in person to cut it out. This may involve the QA spending more time explaining fully but it will save time in future. If a developer is unsure or really clueless they have to be honest and brave and ask! As long as you've tried to understand the problem and put in time people will help you!
• Help each other out you're working as a team, if on a particular day you need more reviewing or the QA needs to concentrate on their own work then amicably agree to compromise and if frequent issues arise report to manager.
• All pulls should be passing CI and have no merge conflicts Never assign something with failing tests unless good reason and be wary of new PRS going into master that may mean there are merge conflicts. A QA will normally instantly assign back.
• Run code before assigning As a QA i run project and instantly try to break the new feature within 10 seconds and lots of time succeed! The developer should be doing this and be trying to find the bugs before review. Especially when there are very obvious ones it becomes very frustrating for QA to have to spend time documenting and reporting them because a developer has been careless. Console logs and warnings should never be present and its an instant sign the developer hasn't played around with their new feature.
• Consider allowing developer to merge If some changes are very small and fully understandable by the developer it can be much more efficient if the developer can address them and merge PR themselves rather than wait another day for QA to do so. This should be discussed and is not perfect process but common sense should ultimately rule.
• Developer should be honest and transparent with imperfections If the developer knows their code is not perfect then make the QA aware rather than waste the QAs time saying things the developer already knows.
• Its ok to not address all QA comments As long as the PR doesn't cause massive breakages and improves the project on the whole its good to get the code in sooner rather than later. However any comments that haven't be addressed, or known bugs should be made as issues and clearly identified so nothing is forgotten about.
• PR descriptions and commit messages Schema for both should be agreed as a team. Issues the code addresses should always be stated in both. A brief summary is always nice but the detail should lie primarily in the issues. Other things like adding WIP to PR titles, or writing notes of temp PR status in the description can be helpful.
• The review process should be reviewed! If you feel something is not right then it should be said. QA is such an important thing and works differently for each team. Refining and adapting process constantly to keep productivity and happiness is key!

[jrans]

@iteles @nelsonic @Danwhy @SimonLab There you go finally! If you read over and see a typo, badly worded sentence, grammar then please do feel free to edit. I will proof read now (as I suggest) but has taken a while to write this. If this wants to be put elsewhere, added to, cut or completely thrown away then be my guest!

[iteles]

@jrans, this is AMAZING, thank you so much!

[nelsonic]

@jrans wow! 😮

[Jbarget]

My thoughts on the QA process:

• As a person with informal QA experience (just reviewing colleague's PRs) I found it hard to know what was expected of me as a dedicated QA on a project

• The line between what my personal preference is and what would be best for the codebase is a hard one to judge. In the end, if I felt a particular point would revolve around my personal preference I didn't mention it

• Bringing up points I would feel conscious of just being annoying, I had to actively remind myself that QA and developers are on the same team and it's good for their work to be critiqued (I think it gets harder to tell yourself that with each PR that you end up sending back for changes)