[Avocado-devel] Collaboration Workflow

Fri Jun 3 11:09:43 UTC 2016

On 06/03/2016 12:30 PM, Lukáš Doktor wrote:
> Hello Amador,
>
> thank you for your workflow. I have some bits to add...
>
> Dne 31.5.2016 v 11:09 Amador Pahim napsal(a):
>> Hello,
>>
>> We are receiving a good number of Pull Requests from new contributors
>> and this is great.
>>
>> In order to optimize the time spent on code reviews and also the time
>> the code writers are investing in adjust the code according to the
>> reviews, I'd like to expose my own workflow that I believe is close to
>> the workflow used by the others full-time avocado developers.
>>
>> The hope is that the new comers get inspired by this and probably take
>> advantage of it.
>>
>> As the biggest number of PRs are coming to avocado-misc-tests, I will
>> use this repository as example.
>>
>> - Fork the repository.
>>
>> - Clone from your fork:
>>
>>  $ git clone git at github.com:<username>/avocado-misc-tests.git
>>
>> - Enter directory:
>>
>>  $ cd avocado-misc-tests/
>>
>> - Setup upstream:
>>
>>  $ git remote add upstream
>> git at github.com:avocado-framework/avocado-misc-tests.git
>>
>> At this point, you should have your name and e-mail configured on git.
>> Also, we encourage you to sign your commits using GPG signature:
>>
>> http://avocado-framework.readthedocs.io/en/latest/ContributionGuide.html#signing-commits
>>
>>
>>
>> Start coding:
>>
>> - Create a new local branch and checkout to it:
>>
>>  $ git checkout -b my_new_local_branch
>>
>> - Code and then commit your changes:
>>
>>  $ git add new-file.py
> I could recommend `git cola` or `git citool` in case you need to add
> several files or split the patch in multiple patches.

This is new for me. I'm using "git commit -a" to determine the hunks I 
want to include in a given commit.

>
> Related note here, each patch should only contain one of the change:
>
> * a new feature
> * style fixes
> * a refactoring
>
> for multiple changes one should split the patch into multiple patches.
> (this is where `git cola` comes in handy)
>
>>  $ git commit -s (include also a '-S' if signing with GPG)
> The `git commit -as` to include all tracked files changes is also quite
> useful ;-)

Yes... as you said, -a works only for already tracked files, but it is 
useful indeed.

>
>>
>> Please write a good commit message, pointing motivation, issues that
>> you're addressing. Usually I try to explain 3 points of my code in the
>> commit message: motivation, approach and effects. Example:
>>
>> https://github.com/avocado-framework/avocado/commit/661a9abbd21310ef7803ea0286fcb818cb93dfa9
>>
>>
>>
>> If the commit is related to a trello card or an issue in github, I also
>> add the line "Reference: <url>" to the commit message bottom. You can
>> mention it in Pull Request message instead, but the main point is not to
>> omit that information.
>>
>> - If working on 'avocado' repository, this is the time to run 'make
>> check'.
>>
>> - Push your commit(s) to your fork:
>>
>>  $ git push --set-upstream origin my_new_local_branch
> I use `origin` for the upstream qemu and "ldoktor" for my repo. Then I
> could sipmly `git push ldoktor branch` or `git push origin branch`
> depending on what I need to do.

I'm so used to have 'origin' pointing to my fork that I'd probably mess 
something if I change 'origin' to point to the main project repo. Also, 
when you clone your fork, origin is pointing to it by default, so the 
only additional configuration is to set the 'upstream' repo. Anyway, 
it's a personal choice.

>
>>
>> - Create the Pull Request on github.
>>
> Always wait for the travis CI, force-push fixes and wait once it passes.
> Personally I ignore pull requests which does not pass the CI unless one
> asks for help mentioning my name.
>
>> Now you're waiting for feedback on github Pull Request page. Once you
>> get some, new versions of your code should not be force-updated.
>> Instead, you should:
>>
>> - Close the Pull Request on github.
> I prefer to keep it open until I push the next version. One can get a
> further recommendations. Additionally I'm trying to reply on the
> questions/demands and usually before sending the v2 I go through the
> comments, check I fixed that and mention something like "fixed" or "ack"
> on the old PR. That way I don't forget to fix things and the reviewer
> knows it'd been taken care of that.
>
>>
>> - Create a new branch out of your previous branch, naming it with '_v2'
>> in the end (this will further allow code-reviewers to simple run '$ git
>> diff user_my_new_local_branch{,_v2}' to see what changed between
>> versions):
>>
>>  $ git checkout my_new_local_branch
>>  $ git checkout -b my_new_local_branch_v2
> I'm too lazy for `_v2`, `my_new_local_branch2` is the way I go :D
>
>>
>> - Code and amend the commit. If you have more than one commit in the PR,
>> you will probably need to rebase interactively to amend the right
>> commits.
> Don't forget to rebase it on top of the latest (updated) master. Also
> switching commits without --ignore-date corrupts the output in GH, so
> I'd recommend:
>
>     git rebase --ignore-date origin/master
>
> before the push.
>
>>
>> - Push your changes:
>>
>>  $ git push --set-upstream origin my_new_local_branch_v2
> I believe `git push origin my_new_local_branch_v2 should suffice.
>
>>
>> - Create a new Pull Request for this new branch. In the PR message,
>> point the previous PR and the changes this PR introduced when compared
>> to the previous PRs. Example of PR message for a 'V2':
> I'd stress out the __changes__ section. It's really hepful.
>
>>
>> https://github.com/avocado-framework/avocado/pull/1228
>>
>> After your PR gets merged, you can sync your local repository and your
>> fork on github:
>>
>>  $ git checkout master
>>  $ git pull upstream master
>>  $ git push
> I have to admit I'm used to `git fetch && git reset --hard
> origin/master` myself to avoid left-overs in case I corrupt the master
> manually...
>
>>
>> That's it. That's my personal workflow, what means it probably differs
>> from what others developers are used to do, but the important here is to
>> someway cover the good practices we have in the project.
>>
>> Please feel free to comment and to add more information here.
> There are few scripts I'm using to simplify my workflow, maybe they'd be
> interesting to some of you. I spent just a little time on them, so they
> are hardcoded, written in bad way, but well... could be used for
> inspiration:
>
> https://github.com/ldoktor/scripts/tree/master/avocado

Thank you for that :)

>
> When I have time, I'd like to include some of the features from the
> scripts to `inspekt`. I really like that program, but suffers some
> issues due which I moved to those scripts. But back in the old
> (autotest) days I was using the `inspekt github` feature heavily.
>
> Regards,
> Lukáš
>
>>
>> Best,
>
>
>
> _______________________________________________
> Avocado-devel mailing list
> Avocado-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/avocado-devel
>