Page tree
Skip to end of metadata
Go to start of metadata

This workflow should be followed by all people that wish to contribute code to Apache Daffodil. There are other types of contributors (e.g. wiki, mailing list support, testing) that this does not cover.

  1. Search for an issue in JIRA that represents the change you would like to make or bug to fix. If one does not exist, create a issue. See the Daffodil Issue Tracker information for creating issues and what information/discussions should take place in JIRA.

     

  2. Assign the issue to your self. You may need to request permissions to modify the bug by sending an email to dev@daffodil.apache.org

  3. Visit the Apache Daffodil GitHub and create a fork by clicking on "Fork" in the top right.
     

  4. Clone your new fork. This will be your origin remote:

    $ git clone https://github.com/<username>/incubator-daffodil.git
    $ cd daffodil
  5. Add the ASF upstream repository as a new git remote, calling it asf:

    $ git remote add asf https://github.com/apache/incubator-daffodil.git
    $ git fetch asf
  6. Create a new branch off of the asf/master branch named daffodil-XYZ-description, where XYZ is the JIRA bug number and -description is an optional, very short description of the bug making it easier to differentiate between multiple development branches. For example:

    $ git checkout -b daffodil-123-bitorder-feature asf/master
  7. Make changes to the branch, frequently adding new commits. Code changes should follow the Daffodil Code Style Guidelines and should add appropriate tests using the Test Data Markup Language (TDML) or unit tests. Tests in src/test/scala-debug that are fixed should be moved into src/test/scala.

  8. When changes are complete, rebase your commits onto asf/master and verify that all tests pass:

    $ git fetch asf
    $ git rebase asf/master
    $ sbt test


    Note that you should not use git pull or git merge to sync to the asf repo. Always fetch/rebase and avoid merge commits. Pull requests containing merge commits will be rejected.

     

  9. If multiple commits were made, git rebase -i asf/master should be used to interactively rebase and squash the commits into the smallest number of logical commits. Most commonly this should be a single commit, but there may be some rare cases where multiple commits make sense.

    Ensure each commit has an appropriate and descriptive commit message. The first line of a commit message should contain a short (~50 characters) description of the commit. The second line should be blank, followed by a longer description of the change, wrapped at 72 characters. This long description should describe what was changed in the commit and, more importantly, why those changes were made. The 'what' can be determined by inspecting the code, but the 'why' is often less obvious. At the end of the commit should be a blank line followed by a reference to the JIRA bug, e.g. DAFFODIL-123. Multiple bugs referenced in a single commit should be separated by a comma on the same line. An example of a commit message is:

    Add support for the dfdl:bitOrder feature
     
    Longer explanation of what changes were made to support the bitOrder
    feature, including a description of why the changes were made. Multiple
    lines are wrapped at 72 characters
     
    DAFFODIL-123
  10. Push your branch to your fork:

    $ git push origin daffodil-123-bitorder-feature
  11. Use the GitHub interface to create a pull request for your new branch.

  12. Wait for review comments. There must be at least two +1's from other committers before the change can be merged. If there are any review comments that require changes or the automated Travis CI build fails, create a new commit on your branch (do not squash your changes yet or use git commit --amend) and push your branch to GitHub for furthur review. The pull request will automatically update with your new commit. Continue this process until at least two +1's are recieved from comitters.

  13. Once at least two +1's are received from committers, a committer can accept the pull request. If you made extra commits in step 12, you should now fetch the latest asf, rebase and squash the changes into a single commit  (fixing potential conflicts), and push to origin using the --force option:

    $ git fetch asf
    $ git rebase -i asf/master
    $ git push --force origin daffodil-123-bitorder-feature
  14. A committer can now merge the pull request using the GitHub GUI. This is to be done clicking the "Merge pull request" drop down and selecting "Rebase and merge". The "Create merge commit" and "Squash and merge" options should not be used.  For new committers, you may need to link your GitHub and ASF accounts by visiting https://gitbox.apache.org before you can merge.
     

  15. Mark the JIRA bug as "Resolved" and add a comment with the hash that includes the fix.

  16. If you would like to clean up, you can now delete your development branch, either via the GitHub user interface or:

    $ git push --delete origin daffodil-123-bitorder-feature
    $ git branch -D daffodil-123-bitorder-feature
  • No labels

3 Comments

  1. Use case: A developer does large volume of work (worth saving), but for some reason has to stop working on it, and we'd like to hand off that work to someone else. It is not ready to be merged.

    How is this accomplished?

    My assumption is that this developer's fork from the github mirror, becomes someone else's remote who pulls the branches into their fork?

    Is that correct?

    1. Correct. The original developer (DevA) should push their branch to their DevA fork. The new developer (DevB) should add DevA's fork as a remote, fetch the new branch, and follow this workflow, pushing to their own DevB fork when changes are complete. 

  2. I want to fold these learnings into the flow above.

    Many of our patch sets are large. We're still refactoring the internals of Daffodil to improve maintainability and performance. So a patch set might modify 80 files. Generally review for large sets of changes like this requires several iterations of a review-fix cycle.

    So what we learned is that each time you want code reviewed, you want to push (without --force) a single separate new commit to your branch. This commit should not be squashed together with any commit from a prior review, but should squash together all commits for changes since the prior review. Your work may go through several cycles of commit, push, get review comments, make changes to respond to them (doing local commits as often as you want), squash local commmits into a "next review commit", push, repeat.

    Let's say your review-fix cycle takes 3 iterations. Then at the end there should be 3 commits on that branch that become part of the pull-request for review. Each commit gathers comments as part of reviewing, and the response to those comments is a new separate commit on the branch.

    Many developers use a "commit often" discipline. Those commits are to your local clone of your fork repository. Commit as often as you want there. When it is time to code review, squash all the commits together into a single commit of changes since the prior review.

    • It's very important that each time you add a new commit for review, that it be separate, not squashed into anything already reviewed before it. This preserves the commentary on prior commits for your pull request. It allows reviewers to see how your new changes addressed the prior comments.
    • It's very important that you will never need to "git push --force ...." anything during the review-fix cycle. If you do, you have done something wrong - like squashed reviewed-commits together with post-review ones.

    When review comments from two reviewers come back +1, then it is time to incorporate the change into the master branch.

    At this point, squash together all the review commits so you have one commit.

    This one you must 'git push --force' to your branch on origin (aka your fork repo).

    • Note: doing this will make it impossible to revisit review comments from the commits that have been squashed together - basically the commentary is lost once the commit is squashed together.

    That code-review UI where you can see changes and comments interleaved into the code,... that UI no longer can retrieve those comments once the commits they are on have been squashed together. 

    That means review comments are not a matter of permanent record - though every one is sent to the dev mailing list, so they're recorded in that way, but you won't be able to open the pull request and revisit comments on the individual review commits any longer.

    This means we need to follow this policy:

    • If a comment contains any description/discussion that wants to be maintained/remembered, it should be edited into a comment in the code (perhaps with a TODO or FIXME tag so it's easy to find.)

    That means when doing code review - it's useful to remind contributors to put things into code comments.