Wilder, Your report have been very extensive. they are useful for the impressive works you contribute. maybe they are a bit overwhelming for the beginner that contributes little improvements. I would like to change the * and - list item denoters into + or - to signify optionality. I think for a simple fix (typo's and such) at the most 3 +s should remain.
You overall idea is good and the frustration from which it arose very valid. €0,02 Op wo 3 jun. 2015 om 10:07 schreef Wilder Rodrigues < wrodrig...@schubergphilis.com>: > Hi all, > > Since I became committer I have been doing a bit more of reviews/tests on > PRs with the goal to increase CloudStack quality. Sometimes it’s a bit > difficult to review/test a PR due to lack of informations concerning the > author's environment, tests that were executes, steps to tests the PR, etc. > So, here I come, with a proposal to have a template for PRs. > > A PR should contain: > > * A title including the ACS issue ID - if any > * A clear description of what has been fixed and how > * A description of the environment where the PR has been tested by the > author of the PR > - KVM/XenServer/HyperV/etc > - Operating System > - Storage type > - Is hypervisor + Manegement Server + Database in the same machine? > > * The tests that have been executed on that same environment by the author > of the PR > - Full Maven build including tests execution? > - New unit tests added? (only if Java code was changed) > - New integration tests added/needed? > - Any integration test executed? If so, which ones? > - Any manual tests? > > * Proposal of the steps to be taken by the reviewers of the PR in order to > test it > - How to reproduce the behvaiour before the the PR > - How to test the changes after applyting the PR > > The list above might look a bit too much. But this will help the reviewers > to get confidence on the changes being proposed. > > In addition, I would like to suggest that if a PR does not contain at > least: > - test environment details > - full Maven build + unit tests > - manual tests to prove that the fix works > - it should not be merged - unless the change is about a typo or a shell > script line that can be tested on a terminal console. > > What are your thoughts on that? > > Cheers, > Wilder
