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

docs: add branch naming conventions #3035

Merged
merged 1 commit into from
Aug 19, 2016
Merged

Conversation

whyrusleeping
Copy link
Member

I'd like to discuss standardizing our branch naming strategy. I've outlined what I aim for when creating branches, but if anyone else has thoughts on this please chip in, i'd love to hear pros and cons.

What i'm aiming for is,
A branch name should

  • tell you immediately what type of change it is making
  • give you a rough idea of what part of the codebase is being changed
  • strive to be concise, screen real-estate is valuable

License: MIT
Signed-off-by: Jeromy why@ipfs.io

@whyrusleeping
Copy link
Member Author

@JesseWeinstein
Copy link
Contributor

LGTM. Might be nice to give a bit more guidance or examples about the subsystem part.

@chriscool
Copy link
Contributor

Yeah, in Git development, Junio Hamano names branches starting with the submitter's initials like "cc/apply-am" for my branch that touches "git apply" and "git am" or "jk/pack-objects-optim" for Jeff King's git pack-object optimization.

But as in GitHub the submitter can easily be seen, I am not sure this as important for IPFS as for Git.

One nit: maybe doc/ instead of docs/ to be consistent with test/ that has no s at the end.

@chriscool
Copy link
Contributor

Also for changes that touch many areas, like adding a parameter to a function that is used in a lot of places, there is no single part of the code that is changed. So some examples about what should be done in such cases are welcome.

@RichardLitt
Copy link
Member

Totally on board with this.

I have a tool - tj's git-extras - which uses the term feature instead of feat, so I just type: git feature something-to-do and it opens up a git branch with the name feature/something-to-do. Can I keep using feature? I can see about getting an alias - feat merged.

Another thing I see often, which I love and do somethings, is to put the issue number in the branch name. For instance, feature/113-fix-header. That way, you know what issue it should be tied to, which helps with making better commit messages.

@RichardLitt
Copy link
Member

I would discourage multiple /s in the branch name - just keep it at feature/dht-nil-providers. The semantic space could easily multiple until you have a lot of areas (like dht), which won't help with organization overall, I think.

@whyrusleeping
Copy link
Member Author

I very much prefer feat over feature, the extra three characters are unnecessary and could be used to provide extra clarity in the rest of the branch name. The emphasis here is on being concise.

I would discourage multiple /s in the branch name

Fair enough, That makes sense to me.

put the issue number in the branch name

This makes sense to me, but i'm not sure if i agree with it completely. The reason being that given a list of branches, i dont have any clear idea what exactly issue 113 is without looking it up. If we instead added another short keyword it would make it much easier to visually tell which branch youre looking for. I could be wrong on this though, i'm open to more opinions.

@dignifiedquire
Copy link
Member

put the issue number in the branch name

Please don't do this, I have used this convention before and it just makes things super cryptic in most cases as nobody remembers all the issue numbers.

@Kubuxu
Copy link
Member

Kubuxu commented Aug 4, 2016

I disagree with not using /s in branch names, they are separators are any but they are different from - in a sense that they feel to introduce more separation.

fix/blockstore/test-race looks much cleaner than fix/blockstore-test-race, in which the subsystem and short description melt together with each other.

@whyrusleeping whyrusleeping added the need/review Needs a review label Aug 4, 2016
@RichardLitt
Copy link
Member

Cool! Different opinions.

Switching to feat sounds good.
Not using issue numbers sounds OK, but I think it is useful sometimes. I don't think this should be mandatory.
Fair point about using / to name better. I would just make sure we keep the first one reasonable. Let's try and iterate?

@jbenet
Copy link
Member

jbenet commented Aug 4, 2016

Sounds good to me.

Only word of caution: implement this as a flexible SHOULD convention, not
MUST. MUSTs increase contrib friction by adding random other knowledge. We
should save MUSTs to really critical things (like CI and verifying
signoffs, etc).
On Thu, Aug 4, 2016 at 15:32 Richard Littauer notifications@github.com
wrote:

Cool! Different opinions.

Switching to feat sounds good.
Not using issue numbers sounds OK, but I think it is useful sometimes. I
don't think this should be mandatory.
Fair point about using / to name better. I would just make sure we keep
the first one reasonable. Let's try and iterate?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#3035 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIcofWJ2T-nrHbivhcV6TkTjaPj48Mjks5qcj5PgaJpZM4Jb2rh
.

@JesseWeinstein
Copy link
Contributor

I want to also discourage using issue numbers in branch names -- better to use them in Pull Requests titles (or descriptions) -- the branch names should be descriptive.

@whyrusleeping
Copy link
Member Author

alright, pushed some changes, RFM?

A few examples of good branch names:

- `feat/cmds/object-diff`
- For a Pull Request that add an `ipfs object diff` command.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that adds.

@RichardLitt
Copy link
Member

LMostlyGTM. Two small grammar nitpicks.

License: MIT
Signed-off-by: Jeromy <why@ipfs.io>
@JesseWeinstein
Copy link
Contributor

LGTM

@whyrusleeping whyrusleeping merged commit 7397dd3 into master Aug 19, 2016
@whyrusleeping whyrusleeping deleted the docs/branch-naming branch August 19, 2016 15:28
@ghost ghost mentioned this pull request Dec 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/review Needs a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants