You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@opennlp.apache.org by kinow <gi...@git.apache.org> on 2017/06/22 07:05:44 UTC

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

GitHub user kinow opened a pull request:

    https://github.com/apache/opennlp-site/pull/21

    OPENNLP-1045: Add Git development page (adapted from Mahout)

    This is an alternative to #11, which follows the suggestion from @smarthi, and adapts the page from the Mahout page on Git and Pull requests.
    
    In case this version is accepted, I will manually close #11.
    
    Cheers
    Bruno

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/kinow/opennlp-site OPENNLP-1045-2

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/opennlp-site/pull/21.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #21
    
----
commit 2e5a9d34872db9766fef1407b1d9aea502b3eb63
Author: Bruno P. Kinoshita <br...@yahoo.com.br>
Date:   2017-06-22T07:04:20Z

    OPENNLP-1045: Add Git development page (adapted from Mahout)

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] opennlp-site issue #21: OPENNLP-1045: Add Git development page (adapted from...

Posted by chrismattmann <gi...@git.apache.org>.

Github user chrismattmann commented on the issue:

    https://github.com/apache/opennlp-site/pull/21
  
    something to consider once in Gitbox if the VOTE passes:
    
    http://wiki.apache.org/tika/UsingGit
    http://wiki.apache.org/nutch/UsingGit



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r123438951

We should not say "you can merge and push them directly"

---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r123439872

--- Diff: src/main/jbake/content/using-git.ad ---
@@ -0,0 +1,178 @@
+////
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+////
+= Using Git
+:jbake-type: page
+:jbake-tags: maven
+:jbake-status: published
+:idprefix:
+
+There are several ways to setup Git for committers and contributors. Contributors can safely setup Git any way they
+choose but committers should take extra care since they can push new commits to the master at Apache and various
+policies there make backing out mistakes problematic. Therefore all but very small changes should go through a PR,
+even for committers. To keep the commit history clean take note of the use of `--squash` below when merging into
+`apache/master`.
+
+## Git setup for Committers
+
+This describes setup for one local repo and two remotes. It allows you to push the code on your machine to either your
+GitHub repo or to Git at Apache (i.e. git-wip-us.apache.org). You will want to fork GitHub's apache/opennlp to your own
+account on GitHub, this will enable Pull Requests of your own. Cloning this fork locally will set up "origin" to point
+to your remote fork on GitHub as the default remote. So if you perform "git push origin master" it will go to GitHub.
+
+To attach to the apache Git repo do the following:
+
+ git remote add apache https://git-wip-us.apache.org/repos/asf/opennlp.git
+
+To check your remote setup:
+
+ git remote -v
+
+You should see something like this:
+
+ origin https://github.com/your-github-id/opennlp.git (fetch)
+ origin https://github.com/your-github-id/opennlp.git (push)
+ apache https://git-wip-us.apache.org/repos/asf/opennlp.git (fetch)
+ apache https://git-wip-us.apache.org/repos/asf/opennlp.git (push)
+
+Now if you want to experiment with a branch, this by default points to your GitHub account because "origin" is default.
+You can work as you normally do using just GitHub, until you are ready to merge with the Apache remote repository.
+Some conventions will integrate with Apache JIRA ticket numbers.
+
+ git checkout -b opennlp-xxxx #xxxx typically is a JIRA ticket number
+ #do some work on the branch
+ git commit -a -m "doing some work"
+ git push origin opennlp-xxxx # notice pushing to **origin** not **apache**
+
+Once you are ready to commit to the Apache remote you can merge and push them directly, or better yet create a
+pull request (PR).
+
+## How to create a PR (committers)
+
+Push your branch to GitHub:
+
+ git checkout opennlp-xxxx
+ git push origin opennlp-xxxx
+
+Go to your opennlp-xxxx branch on GitHub. Since you forked it from GitHub's apache/opennlp it will default any PR to
+go to apache/master.
+
+* Click the green "Compare, review, and create pull request" button.
+* You can edit the _to_ and _from_ for the PR if it is not correct. The "base fork" should be apache/opennlp unless
+you are collaborating separately with one of the committers on the list. The "base" will be master. Do not submit a
+PR to one of the other branches unless you know what you are doing. The "head fork" will be your forked repo and the
+"compare" will be your opennlp-xxxx branch.
+* Click the "Create pull request" button and name the request "OPENNLP-XXXX" (uppercase). This will connect the
+comments of the PR to the mailing list and JIRA comments.
+* From now on the PR lives on GitHub's apache/opennlp. You can use the commenting UI there.
+* If you are looking for a review or sharing with someone else say so in the comments but do not worry about
+automated merging of your PR -- you will have to do that later. The PR is tied to your branch so you can respond to
+comments, fix code, and commit from your local repository. They will appear on the PR page and be mirrored to JIRA
+and to the mailing list.
+
+When you are satisfied and want to push it to Apache's remote repository proceed with **Merging a PR**
+
+## How to create a PR (contributors)
+
+Create pull requests: <<anchor-1,[1]>>.
+
+Pull requests are made to apache/opennlp repository on GitHub. In the GitHub UI you should pick the master
+branch to target the PR as described for committers. This will be reviewed and commented on so the merge is
+not automatic. This can be used for discussing a contributions in progress.
+
+## Merging a PR (yours or contributors)
+
+Start with reading <<anchor-2,[2]>> (merging locally).
+
+Remember that pull requests are equivalent to a remote GitHub branch with potentially a multitude of commits.
+In this case it is recommended to squash remote commit history to have one commit per issue, rather
+than merging in a multitude of contributor's commits. In order to do that, as well as close the PR at the
+same time, it is recommended to use **squash commits**.
+
+Merging pull requests is equivalent to a "pull" of a contributor's branch:
+
+ git checkout master # switch to local master branch
+ git pull apache master # fast-forward to current remote HEAD
+ git pull --squash https://github.com/cuser/opennlp cbranch # merge to master
+
+`--squash` ensures all PR history is squashed into single commit, and allows committer to use his/her own
--- End diff --

But this also overwrites the authors, so we should give credit to the original contributor. I suggest we use an approach where we rebase first and then merge.

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by jfrazee <gi...@git.apache.org>.

Github user jfrazee commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r124300001

@kinow @kottmann I think it might be worth considering using `git am --signoff` on top of this. So contributor rebases and squash against master just prior to merge (sometimes having the multiple commits during review is helpful) and then the merge by a committer is with `--signoff` which ensures the contributor gets credit, but also the git history reflects who signed off and performed the merge.

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r123439516

We should suggest to use git commit --amend and git push -f to to react on feedback

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r123442070

Doing git rebase and merge is a simpler process because the name/email and commit message doesn't need to be copied by hand.

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by jfrazee <gi...@git.apache.org>.

Github user jfrazee commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r124310132

@kottmann I think I wasn't clear. The signoff is only for committers/person doing the merge so there's a barrier to entry but I think it's fair to stretch them a little further than you would a contributor.

Yeah, the rebase can be confusing as heck for even very competent developers who don't use git a lot, so I think it's worth recommending for contributions in the PR but that's something the committer/merging person can always cleanup when they're doing their thing. Burden is on the committers really and whether they want to do it themselves or teach the contributor.

[GitHub] opennlp-site issue #21: OPENNLP-1045: Add Git development page (adapted from...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on the issue:

    https://github.com/apache/opennlp-site/pull/21
  
    @kinow this will now change a bit again due to the migration to GitHub, I propose we hold it off another week. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r123439326

Maybe nicer to have the command to set the upstream, otherwise you have to write everytime origin.

[GitHub] opennlp-site pull request #21: OPENNLP-1045: Add Git development page (adapt...

Posted by kottmann <gi...@git.apache.org>.

Github user kottmann commented on a diff in the pull request:

https://github.com/apache/opennlp-site/pull/21#discussion_r124306989

I personally think it is best if the contributor just gives us the commits we can include without any modification in master (expect a rebase) in a PR.

@jfrazee do you think this makes a too high entry barrier for new contributors? One problem I see with that is that it requires some git skills, and if people don't know it well they have a hard time doing that.

[GitHub] opennlp-site issue #21: OPENNLP-1045: Add Git development page (adapted from...

Posted by kinow <gi...@git.apache.org>.

Github user kinow commented on the issue:

    https://github.com/apache/opennlp-site/pull/21
  
    Happy to wait @kottmann and to amend the text as necessary. Not sure if others can edit this PR as this is in a branch in my fork, but if so, feel free to re-word the text as appropriate too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---