Guidelines for Committing Code to DSpace

...

GitHub

...

1. Use JIRA to track your changes
   • Ensure there is an issue describing the fix/improvement/feature in JIRA (If this is a minor typo fix, or an obvious bug, you need not add a JIRA issue – use your judgment on whether this fix is worth tracking in JIRA or not)
2. Share Submit a Pull Request for your code before committingchanges
   • First off, you should be doing the majority of your development in your own GitHub Fork. See Development with Git.
   • Generally speaking, code changes should come in via a Pull Request. This gives the rest of the Committers/Developers a chance to comment on the Pull Request
   • If this is a more significant fix/redesign, you may want to use the SVN sandbox to manage your code (rather than dealing with large patches, etc). You also may want to document your work on the Wiki, and link both of these back to the JIRA issue.
   • If this is a smaller bug very minor fix/change , you could just work on your code locally and generate a patch to attach to the JIRA issue(typo/improved comments/obvious bug), you can bypass creating a Pull Request and commit it immediately. This is an exception to the rule.
3. Code approval Code "approval" processes
   • General Rule of Thumb: Committers are trusted developers. Use your best judgment on whether or not you feel a particular piece of code should be approved before committing/merging your Pull Request. The below guidelines are just suggestions and they may not apply to all scenarios:
   • If this is a small, obvious fix (typo/improved comments/obvious bug), just commit it immediately. This is the exception to all other rules – there's no reason to add a JIRA issue for every typo or obvious fix, and no reason to wait around for approval to commit it.
   • If this is a bug/issue fix, we generally try to run it by get approval for your Pull Request from at least one other committerCommitter. If there is any doubt, feel free to run it by others or ask to have it added to the agenda of the next developers meeting.
   • If this is a new feature, we generally try to run it by a few committers generally speaking you need at least three Committers to approve (or bring it up in a developer meeting, if it's a larger change). The only reason we suggest to run it by more than one committer is that oftentimes others may be able to provide additional suggestions or configurations that may be worth investigating.
   • Please note that our Developer Voting Procedures actually state that at least three Committers should approve of all code changes, and none should disapprove. We tend to be lenient on minor code changes / bug fixes (as these tend to be non-controversial changes). But, new Features should really get the approval of at least 3 Committers.
4. Document your code early & often
   • New features/improvements need to come with documentation, otherwise we won't be able to accept them. Keep this in mind while you are developing, and use the Wiki or JIRA to start your documentation early on (it'll make it easier on you in the long run).

...

1. Once Approved, Merge your Pull Request in a timely manner
   • Once your Pull Request is approved, you should work to merge it as soon as you can.
   • Before merging, please ensure that your code doesn't break any Unit Tests. This will save you a potential headache later, when our Continuous Integration system does a verification of all Unit Tests.
   • If the Pull Request was not created by a Committer, the Committer(s) who are most familiar with the work (and who may have been supporting/giving feedback) should merge the Pull Request.
   • Obviously, Committers can ask for someone else to take over the merge process as needed.

GitHub "Master" Committing Rules

The GitHub Master Branch The SVN Trunk is one of the few places that needs to remain well managed. As many committers work out of it on an almost daily basiswe all fork it and/or create Pull Requests from it, it can be detrimental to us all if incomplete or extremely buggy code is committed. Obviously, mistakes happen (and we've all made them), so don't worry if you accidentally commit/merge something you didn't mean to (just try to roll it back as soon as you notice it). Here are the few rules we try to follow when committing code to TrunkMaster Branch:

1. No incomplete features in on the "TrunkMaster Branch", ever.
   • If you are in-progress on a feature, create an SVN sandbox or DSpace branch area to work on it, and pull it over to Trunk once it has been completed and is ready for testing & release. This will hopefully help us to avoid encountering a situation where we need to revert a large, unfinished patch at the last moment.develop it in a branch or fork (see Developing with Git) and submit a Pull Request once it is ready for feedback. (NOTE: You need not necessarily wait until it is complete to create the initial Pull Request, since Pull Requests can be updated easily. But, obviously the Pull Request should not get merged until the code is complete and approved.)
   • All DSpace GitHub projects (i.e. not just DSpace/DSpace) should Modules which do not reside in DSpace "Trunk" (e.g. http://scm.dspace.org/svn/repo/modules/) should also follow this rule. If there is major rework to be done on a module, create a " branch " or fork to do that the work and migrate that branch back to the Module's Trunk once it is ready for release, and then submit it via a Pull Request.
   • Clarification: Although this rule may seem strict, it is meant to point out that a broken Trunk "Master" branch is an urgent issue. There may be situations where breaking Trunk the Master branch temporarily may even be necessary. However, we should make all attempts to keep breakage to a minimum (or forewarn the Committer team if anticipated). Temporary breakage is OK, but longer-term breakage (a few days or more) will can negatively affect us all.
2. All new features must have documentation before committing to Trunkmerging into the Master Branch.
   • New features/improvements should not be committed to Trunk merged/committed into the Master Branch until there is some minimal documentation (minimal documentation includes documenting all configuration options). Ideally, they should also come with usage documentation (i.e. examples of how to use the feature, how to configure the feature, etc.) This will help us all ensure that Documentation is ready by the time we get to the next release, and hopefully lessen the time that any one person has to spend cleaning up or rewriting documentation.