cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [DISCUSS] Template to be used for Pull Requests
Date Wed, 03 Jun 2015 11:19:18 GMT
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 <
WRodrigues@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message