[Avocado-devel] Collaboration Workflow

Fri Jun 3 10:30:16 UTC 2016

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.

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 ;-)

>
> 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.

>
> - 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

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,

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/avocado-devel/attachments/20160603/03d5037c/attachment.sig>