search

evertrue / zookeeper-cookbook

Chef cookbook for installing and managing Zookeeper.
https://supermarket.chef.io/cookbooks/zookeeper
Apache License 2.0
81 stars 117 forks source link

Move zookeeper mirror attribute to zookeeper recipe #78

Closed jakedavis closed 10 years ago

jakedavis commented 10 years ago

Reported in #77. Will bump to 1.7.2 and merge into master directly.

jeffbyrnes commented 10 years ago

@jakedavis just a heads-up, the way this merged in doesn't line up with the version it supposedly belongs to; 352295bc05583910ef897e7cb6d1a91ebed50ae7 comes before the merge of this branch/PR (contrary to its commit message).

We've found the best practice is this workflow:

  1. Branch for feature/fix
  2. Create PR for said branch & review
  3. Merge & close PR
  4. Create a new commit on master, bumping the version number & updating the CHANGELOG
  5. Tag this new commit on master w/ the updated version number
  6. Ship the cookbook to the Chef Server and/or Community Site

Also, it would be great to publish some of these most recent fixes to the community site; the latest version there is 1.6.1.

@eherot & I are happy to help out with any/all of this!

jakedavis commented 10 years ago

Oops, I think what must have happened is I committed 352295b to master as opposed to this branch and then pushing this branch to master. My mistake. Looks like the only time I made that mistake, but it might not have been. I've got the flow down now though so I won't make that error again, thanks for mentioning it.

Tagging is a thing I've never done, but it sounds like demand is high :) so let's do it for sure. Looks like I'm a collaborator on the community cookbook, so I'll try to upload there and add you guys or something. To be honest, we don't use community much, but I admit we should do a better job there. I'll try to figure it out and see if I can't upload the newer versions.

You guys are full time collaborators, so don't feel held back :) I see @eherot is taking on #80, which is super.

jakedavis commented 10 years ago

Okay, I have logged in and it looks like it does offer me the option of uploading to the community site, so I can tackle that hopefully by end of week.

eherot commented 10 years ago

No worries. I don't mind cleaning up the existing tag scheme (I'm almost done anyway). Just in the future please make sure to follow the procedure outlined by @jeffbyrnes above to keep things sane. The most important thing, really, is that changes to the version number and the changelog only be made on master, and then the actual commit that is shipped to the Chef community site should be the one that gets tagged. If this procedure is followed there should not, for example, be any need to put the version number in the commit message, and it should be pretty straightforward to exactly match a given community release with a tag.

Please don't hesitate to ping me or @jeffbyrnes if you have any questions about the source control/tagging process. We are always happy to help. :-)

jakedavis commented 10 years ago

Ah, mostly I version the commit message just because I find the visuals going through commit history easier to follow.

Usually my procedure nowadays is to pull down the to-be-merged PR branch, add a commit like "[1.0.0] Do thing" to that branch, and then push it directly to master (mostly to avoid merge commits, which I find kind of noisy). That falls in line with the procedure above, but I think the mistake I made earlier was pushing the version commit to the branch and then merging via the Github UI. I don't do that anymore so it should be okay. I'll remember to tag and update the changelog in the future though :) thanks again, will definitely reach out with more questions!