Housekeeping: commits to master and PRs

9 views
Skip to first unread message

Antonin Delpeuch (lists)

unread,
Nov 12, 2017, 6:18:38 AM11/12/17
to openref...@googlegroups.com
Hi all,

As the activity on the project increases, I would like to suggest a few
housekeeping rules to ease the development.

I think we should try to avoid empty PRs like these ones:
- https://github.com/OpenRefine/OpenRefine/pull/1322
- https://github.com/OpenRefine/OpenRefine/pull/1318
- https://github.com/OpenRefine/OpenRefine/pull/1315
- …

Pull requests exist so that people can make comments on the changes they
contain (before they are merged on master), so let's use this feature!

In many open source projects, the workflow looks like this:

- the developer pushes commits either to their own fork of the project
or a branch of the root repository. If they use their own fork, they
should also use a branch that isn't the default one (master). This is
just to ensure that they can work on multiple fronts: if, while they are
working on a big feature, they discover an unrelated bug that should
urgently be fixed, they can easily create a separate branch and submit
an independent PR for that.

- the developer submits a PR to merge from the feature branch to master

- the users and other devs test the features *before the changes are
merged in master* by checking out the appropriate branch. If the changes
are in a branch of the root repository, this is very simple: just do
"git pull ; git checkout my_awesome_branch" and try the changes.

- the continuous integration services provide feedback on the PR that
can also be used to improve the PR

- once everybody is happy with the new feature or the bug fix, the PR is
merged, ideally not by the author of the PR

This ensures that the master branch will always be in a clean state.

Any thoughts?
Antonin

Thad Guidry

unread,
Nov 12, 2017, 12:04:04 PM11/12/17
to openref...@googlegroups.com
Yes Antonin, that has always been our practice.
Jacky is just not following that practice all the time :)  
Jacky start following our practice :)

To always work with Pull Requests and avoid merging directly onto our master from contributors.  Even though many of you have access to merge directly to our master, please avoid doing so.  Instead, do Pull Requests once the team is satisfied with your work.

Instead of merging to our master directly (which we don't like and want to avoid),  only do a pull request of your branch from Github.  I would suggest to just create more dev branches for your work on your branch of our master.
For instance, for the metadata enhancement, I would have created a simple local branch called "metadata_enhancement" or similar and pushed to your remote same name branch...then, once the team was satisfied with your remote branch "metadata_enhancement" after we reviewed, tested it out with community, then we would finally merge your branch into our master via a pull request from you. 


Thanks Team !

Thad Guidry

unread,
Nov 12, 2017, 12:11:35 PM11/12/17
to openref...@googlegroups.com
Oops, I re-read and should have clarified...

You do not need to fork the repository, since you are all direct contributors.
But instead simply create a well named branch off of master to work on your feature, like "metadata_enhancement".

master ---*------*-----------*-----------------------PR----->
                \---- metadata_enhancement ---/

Antonin Delpeuch (lists)

unread,
Nov 12, 2017, 12:22:33 PM11/12/17
to openref...@googlegroups.com
Thanks Thad!
Yes, that's the ideal situation.

Antonin
> +ThadGuidry <https://plus.google.com/+ThadGuidry>
>
> --
> You received this message because you are subscribed to the Google
> Groups "OpenRefine Development" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to openrefine-de...@googlegroups.com
> <mailto:openrefine-de...@googlegroups.com>.
> For more options, visit https://groups.google.com/d/optout.

qi cui

unread,
Nov 12, 2017, 5:38:17 PM11/12/17
to OpenRefine Development
The community had not been very active for a while. And different members have different paces and focus. I know that there was some PRs sitting there for quite long time waiting for reviewing but never happened. 

Since the community has been gaining more momentum recently. I think it make sense to have more governance like this. 


> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "OpenRefine Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openrefine-dev+unsubscribe@googlegroups.com.

Thad Guidry

unread,
Nov 12, 2017, 8:04:32 PM11/12/17
to openref...@googlegroups.com
Thanks Jacky.



> For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "OpenRefine Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openrefine-de...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "OpenRefine Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to openrefine-de...@googlegroups.com.

Thad Guidry

unread,
Nov 13, 2017, 4:57:57 PM11/13/17
to openref...@googlegroups.com
Team,

I have set our MASTER branch to Protected Status for only allowing Code Reviewed Pull Requests.
This is to follow our best practice of ensuring only commits that have been Code Reviewed are pulled into our Master branch.

FYI, in case I die...the setting is under our Admin / Branches setting. :)

Thanks All !

Thad Guidry

unread,
Nov 13, 2017, 4:59:46 PM11/13/17
to openref...@googlegroups.com
Oh, 1 more thing.

In doing the Protected Branch... I had to migrate the team from Legacy to Current.

Antonin and Jacky were the 2 members that were not part of Legacy and so your permissions have been slightly adjusted automatically in doing so.
Please let me know if you ever have problems from here on out.  But I don't think so, perhaps other than being restricted on creating new repositories for the Organization.

Thanks.

magdmartin

unread,
Nov 22, 2017, 10:52:24 PM11/22/17
to OpenRefine Development
Feel free to update the https://github.com/OpenRefine/OpenRefine/blob/master/CONTRIBUTING.md document with best practice you want to see for any contributors. There is little chance that a new contributors find this email thread. 

Reply all
Reply to author
Forward
0 new messages