Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guides - Update Developer Guide to Reflect New Process #2863

Closed
pdurbin opened this issue Jan 13, 2016 · 20 comments
Closed

Guides - Update Developer Guide to Reflect New Process #2863

pdurbin opened this issue Jan 13, 2016 · 20 comments
Assignees

Comments

@pdurbin
Copy link
Member

pdurbin commented Jan 13, 2016

It's a new year and we've introduced a new "develop" branch, which is now the default. We need to update the "Branching Strategy" and "Making Releases" docs to explain the purpose of the "develop" branch:

Top of mind thoughts:

  • The "develop" branch represents all the code that will go in the next release (4.2.4 as of this writing).
  • Code changes should be made in branches and tested before they are merged into "develop".
  • We can probably replace references to "release branches" and explain that "develop" is what eventually gets merged into master.

Questions:

In case it's helpful, at the time I wrote the old/current branching strategy doc, we were talking a lot about a pipeline where there's handoff from design to dev to QA and a three week development cycle. Here's the picture I drew back then:

devcycle

@pdurbin pdurbin added this to the 4.3 milestone Jan 13, 2016
@pdurbin pdurbin modified the milestones: 4.2.3, 4.3 Jan 13, 2016
@pdurbin pdurbin changed the title Update "Branching Strategy" doc to reflect new "develop" branch on GitHub Update "Branching Strategy" and "Making Releases" docs to reflect new "develop" branch on GitHub Jan 14, 2016
@bencomp
Copy link
Contributor

bencomp commented Jan 14, 2016

@pdurbin Is there any consensus on the branching strategy at IQSS, or is this just your idea and up for discussion? If PRs keep being closed for making them against branches that are closed because the branching strategy may change, I may very well lose interest in contributing. Moreover, if it's just you (and for just a tiny bit, me) who determine(s) how developers should write code and commit it, what are the odds that this new strategy is implemented and adhered to?

I would like to have this discussion with all developers, because this single issue on GitHub is easily missed by many. Could you (or rather the team lead(s)) start this wider discussion on the community list?

@pdurbin
Copy link
Member Author

pdurbin commented Jan 14, 2016

@bencomp it was nice chatting with you at http://irclog.iq.harvard.edu/dataverse/2016-01-14 and I hope I answered most of your questions! We love our contributors! Please stay tuned!

@bencomp
Copy link
Contributor

bencomp commented Jan 15, 2016

You did answer my main question about consensus on IRC (I admit I was a bit frustrated when I wrote the above comment).
It would be helpful to explain the reasons for the (changed) branching strategy and refer to the team decision. Perhaps add some background about how changes were pushed to master and what problems you ran into?

Regarding your questions:

  • yes, pull requests should be made against a persistent branch (that should prevent having to reopen pull requests when a branch is closed…)
  • a pull request should explain what was changed and for what reason(s). Referring to issues seems good practice to me. If "fixes #issue" is in the subject, merging the PR also closes the issue. Should a Request for Integration include more?
  • I don't know what is best for (user) documentation, it may depend on the context of documentation changes. If a change accompanies a feature, just put it in the same feature branch and/or pull request. If making small changes to formulations or correcting typos, I don't mind pushing them directly to the develop branch.

@pdurbin
Copy link
Member Author

pdurbin commented Jan 15, 2016

Please wait for something more official but my take on the reasons for the change are:

  • We want code to be better tested in a branch before it gets merged into the branch that represents the next release.
  • We want the branch that represents the next release to have a generic name such as "develop" so we can decide closer to release time if it will be a minor or patch release as defined by http://semver.org
  • We want developers to know that they can always branch from "develop" when starting work on a new feature or bug fix. Don't worry about which release it may eventually land in.

That said, we're still discussing. And really, we need to experiment a bit with any new strategy and get our hands dirty before we can say if we're happy with the change. I'm well aware that changing strategies is hard on contributors, so please pardon our dust!

I started working on a pull request section yesterday and I'm reminded of this quote I wrote in the contributing file, which is still what I want:

"Before you start coding, please reach out to us either on our dataverse-community Google Group, IRC, or via [email protected] to make sure the effort is well coordinated and we avoid merge conflicts."

I was starting to think that the emphasis of whatever doc should be not what the branching strategy is but rather clear advice for contributors on how to make your pull request such that it's more likely to be merged.

@pdurbin
Copy link
Member Author

pdurbin commented Jan 15, 2016

@kcondon as we've discussed I made a very small change in b2fcf4d to top of the following pages to note that the branching strategy is under review and that this issue is the one to follow for updates:

I just did a build and these changes are live so I'm passing it to QA. As agreed, if it looks fine let's move this issue to the 4.3 milestone so we can further improve the docs.

@kcondon kcondon removed their assignment Jan 15, 2016
@kcondon kcondon modified the milestones: 4.3, 4.2.3 Jan 15, 2016
@kcondon
Copy link
Contributor

kcondon commented Jan 15, 2016

OK, verified the above changes. Marking the follow on work described by this ticket for v4.3.

@mheppler mheppler changed the title Update "Branching Strategy" and "Making Releases" docs to reflect new "develop" branch on GitHub Guides - Update Developer Guide to Reflect New Process Jan 22, 2016
@mheppler
Copy link
Contributor

I've expanded this ticket to be updating the Developer Guide as a whole, since other pages also needed updated. A draft of the proposed changes is being worked on in Google Docs. As more pages are discovered to require updating, feel free to update this comment to add them to the check list below.

  • Branching Strategy
  • Testing
  • Making Releases
  • Documentation

@pdurbin
Copy link
Member Author

pdurbin commented Mar 29, 2017

I just created pull request #3734 and moved this issue to Code Review at https://waffle.io/IQSS/dataverse

@pdurbin
Copy link
Member Author

pdurbin commented Mar 30, 2017

@mheppler provided some better wording in PULL_REQUEST_TEMPLATE.md which I just added in d86777b

@dlmurphy
Copy link
Contributor

I've made some tiny typo fixes to the updated documents and everything is now looking great to me!

@dlmurphy dlmurphy removed their assignment Mar 30, 2017
@pdurbin
Copy link
Member Author

pdurbin commented Mar 30, 2017

@dlmurphy thanks for catching those typos!

@pdurbin
Copy link
Member Author

pdurbin commented Mar 30, 2017

This shouldn't be a blocker for merging pull request #3734 but I did just solicit feedback from external folks at https://groups.google.com/d/msg/dataverse-dev/6qQVpb8c3zg/r783YQxkBAAJ

I'm moving this to QA at https://waffle.io/IQSS/dataverse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants