[Avocado-devel] Collaboration Workflow

Fri Jun 3 11:10:31 UTC 2016

On 06/03/2016 01:09 PM, Amador Pahim wrote:
> 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.

I meant 'git commit -p'

>
>>
>> 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
>>
>
> _______________________________________________
> Avocado-devel mailing list
> Avocado-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/avocado-devel