Skip to end of metadata
Go to start of metadata

Note: In the below steps, <version_branch> should be replaced with the version branch of Daffodil you are working on based on the Branch Workflow.

 

  1. JIRA bug is created, status is "Open"

  2. JIRA bug is assigned to implementor

  3. Implementor "Starts Progress" in JIRA bug

  4. Implementor creates a new bug branch

    $ git fetch --prune origin
    $ git checkout -b review-sdl-123 --track origin/<version_branch>

    The branch name must use the following naming convention:

    review-xxx-YYY-very_short_desc

    PartDescription
    xxximplementor initials, in lower case
    YYYJIRA bug number, with DFDL prefix removed
    desca short description (optional)

    Note that the JIRA bug number is mandatory. This means all changes must have an associated JIRA bug.

    Also note that the description has been left off in the examples for brevity.
     

  5. Implementor performs all development in bug branch, this includes repeating the following steps many times until you are ready for review:

    $ # edit files
    $ git add <files>
    $ git commit
    $ git fetch --prune origin
    $ git rebase origin/<version_branch>
    $ sbt test
    $ sbt cli # optional step, but should do these tests also
    $ git push --force origin review-sdl-123

    Notice the frequent commits and pushes. This is a great way to ensure we do not accidentally lose commits. But be careful when using the --force option, as that will replace whatever branch is on the server.

    Also, please read A Note About Git Commit Messages for the correct commit message format. Please follow those standards, with the additional requirement that the last line should contain only the JIRA bug number preceded by an empty line, e.g. "\nDFDL-123". If there is more than one bug related to the commit, separate them with commas.

    Additionally, tests should be created during this process to convince yourself that your changes are correct. These tests should be placed in the "src/test/scala-new" directory.

    If this is a bug fix ticket, any tests that were previously failing should be moved from scala-debug to scala-new.
     

  6. When finished developing, implementor ensures changes work with the latest upstream

    $ git fetch --prune origin
    $ git rebase origin/<version_branch>
    $ sbt test
    $ sbt cli # runs CLI tests

        

  7. Implementor squashes commits into a reasonable patchset.

    $ git rebase -i origin/<version_branch>

    Please pay attention to commit message format
     

  8. Implementor ensures all tests pass

    $ sbt test
    $ sbt cli # runs CLI tests

        

  9. Implementor pushes to ncsa opensource servers for review

    $ git push --force origin review-sdl-123

        

  10. Implementor creates review in Crucible and assigns to reviewer

    1. Go to Daffodil Fisheye

    2. Click "Create Review"

    3. Select the "DFDL" project and click "Create Review"

    4. Click "Browse Changesets"

    5. Check the checkmarks next to your commits
       
    6. Click "Edit Details"

    7. Change the author and moderator to Implementor

    8. Add one or more reviewers

    9. Click "Done"

    10. Select "Start Review"

  11. Implementor prepares bug for review, performing the following tasks:

    1. Add comment to Jira issue containing requirement ID's met when implementing the Jira bug

    2. Add Crucible review ID to the comments

    3. Choose "Review Solution", placing JIRA bug "In Review"
          
  12. Reviewer reviews patchset. If changes are necessary:


    1. Reviewer annotates Crucible review with comments

    2. Implementor makes changes to bug branch according to comments (following steps 5-9)

    3. If necessary, implementor pushes bug branch back to ncsa for re-review

    4. Repeat review process as necessary. Note, if changes are simple enough, re-review is not always necessary. Use your own judgment
          
  13. Implementor closes review by selecting "Tools > Close"

  14. Implementor ensures changes work with the latest upstream

    $ git checkout review-sdl-123
    $ git fetch --prune origin
    $ git rebase origin/<version_branch>

        

  15. Implementor squashes review changes into a reasonable patchset

    $ git rebase -i origin/<version_branch>

    Please pay attention to commit message format
        

  16. Implementor ensures all tests pass

    $ sbt test 
    $ sbt cli # runs CLI tests

        

  17. Implementor sets <version_branch> branch to bug branch and pushes

    $ git checkout <version_branch>
    $ git reset --hard review-sdl-123
    $ git push origin <version_branch> # do not use the --force option here 

    If commit was a bug fix or a test on <version_branch> and a newer version branch exists:

     

    1. Implementor merges the change into the newer version branch, and fixes any conflicts:

      $ git checkout <new_version_branch>
      $ git merge <version_branch>
    2. Implementor ensures all tests pass

      $ sbt test
      $ sbt cli # runs CLI tests

      If tests fail due to incorrect conflict resolution, changes should be made and amended to the merge commit until tests pass:

      # make changes
      $ git add <files>
      $ git commit --amend
    3. Implementor pushes changes

      $ git push origin <new_version_branch> # do not use the --force option here



  18. Implementor deletes bug branch (both remote and local)

    $ git push --delete origin review-sdl-123
    $ git branch -D review-sdl-123 

        

  19. Implementor prepares bug for validation, performing the following tasks:

    1. Choose "Review Accepted", marking JIRA bug "Resolved"

    2. Add commit ID to comments

    3. Add names and locations of any new/fixed tests in scala-new to comments

    4. Assign to TL
  • No labels

5 Comments

  1. At step 18, I had to use the --force option when pushing. Otherwise I got non-fast-forward errors back not about master, but about my review branch and about an old obsolete branch.

  2. Are you sure you checked out the master branched first? If you're pushing your review branch, you'll need the --force option. But if you're pushing master, you should never use the --force option. If you use --force when pushing master. It's also possible that someone pushed inbetween the fetch in step 15 and the push in step 18. In which case you'd need to repeat steps 15-18 again (e.g. fetch latest, rebase ontop of it, test, push).

  3. Ah, actually, I think you may need to specify the branch to push. It should be 'git push origin master'. I've updated step 18.

  4. If we're going to make this by the book, we should probably add a section on proper conflict resolution.

  5. An important git practice: if you move a file (change package, or directory, or project...) you should commit without edits to that file.

    This insures that git can find the file and determine that the file was moved from one place to another so that it can retain the history.

    An additional detail is that one should not squash a commit with file moves into another commit, nor squash other commits into a commit with file moves.

    Changes to other files than the one(s) moved can be co-resident with file moves. The point is just not to both move and edit a file in the same commit because of the potential for loss of change history (which can make rebase very hard.)

    So roughly: commit, then move files around, then commit, then get back to edits/changes.