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

Possibility to set branch name? #58

Closed
howardroark opened this issue Jul 8, 2016 · 19 comments
Closed

Possibility to set branch name? #58

howardroark opened this issue Jul 8, 2016 · 19 comments

Comments

@howardroark
Copy link

This looks great! Would entertain the option to set the branch name? I like to use release vs master.

@sindresorhus
Copy link
Owner

Set a branch name for what exactly?

@jamestalmage
Copy link
Contributor

I think he's asking for the ability to set a config where releases must be performed from the release branch.

@howardroark
Copy link
Author

Thanks @jamestalmage ! Yeah... I tend to use develop as the default branch and release as the branch where releases are built and tagged.

@sholladay
Copy link
Contributor

I see two simple solutions to this.

  1. You just put something like alias np='np --any-branch' into your shell's startup file. Not ideal because it applies to every project and skips the branch check entirely, but if this is really common for you, then I don't think it's a big deal.
  2. np gains the ability to be configured via package.json, like XO has. Then you can use --any-branch or some hypothetical more restrictive option, permanently and locally to a project.

@sindresorhus
Copy link
Owner

I would be ok with 2. if someone did a good PR with docs and tests. Should use pkg-conf.

@forivall
Copy link

forivall commented Apr 5, 2019

as I tend to use latest for my release branch name, i'll look into taking on this issue.

@itaisteinherz
Copy link
Collaborator

Note that the 2nd option suggested in #58 (comment) has already been implemented in #354.
@sindresorhus Is there any specific reason why we have a --any-branch flag instead of a more-specific --branch XXX flag?

@sholladay
Copy link
Contributor

I implemented --any-branch and it was my idea, so you can blame me. :)

My reasoning at the time was:

  1. In the vast majority of cases, releases happen from master. And when you allow every little thing to be customized, over time the complexity of our tools explodes. It's also an extra opportunity for the user to make a mistake or misunderstand the feature. For example, --any-branch feels very intuitive to me, whereas --branch XYZ could have a few possible meanings at a glance (e.g. you might expect it to automatically switch you to that branch before release). I felt it was reasonable to be opinionated, with a simple opt-out switch.
  2. In order for the user to benefit from a --branch XYZ option, they'd have to know ahead of time what XYZ should be. As someone who has built a lot of devops tooling for companies, I can tell you that in many cases, custom release branch names are actually derived from the version to be released. So realistically, many users of --branch would also be specifying an explicit version to publish, and they'd have a complex release script built around np to facilitate it. I felt that went against the goals of np.
  3. I wanted to avoid annoying users in the case where they want to do a one-off release from a temporary branch. They shouldn't have to look up what they named their branch and then type it again on the command line just to satisfy np. It seemed better to have a simple way to consistently bypass the check without knowing the branch name.

That said, np has clearly grown beyond its humble beginnings and maybe it makes sense to re-evaluate that decision.

@forivall
Copy link

forivall commented Apr 6, 2019

yeah, in the spirit of that keeping simplicity principle, i was going to just suggest supporting a few well-known keys, such as "release" and "latest" and "current", and to do some light amount of probing to make sure that there was only one of these release-named branches. But as a configuration option has been proposed here, it seems reasonable to propose a configuration based PR.

@itaisteinherz
Copy link
Collaborator

@forivall I don't think we should add support for specific branch names other than master, as everyone has their own preferences for branch naming and such (e.g. gitflow). It seems to me like moving forward, there are two options we can choose from:

  1. Keep the current default configuration and flags, but warn the users of the branch they're publishing from when using --any-branch.
  2. Implement --branch XYZ, as described above.

The option I'd go with is #2, though I'd still like to warn users that they're not publishing from master.

As someone who has built a lot of devops tooling for companies, I can tell you that in many cases, custom release branch names are actually derived from the version to be released.

@sholladay If we go with --branch we could add a wildcard * option, which would be a bit of a hack and defeats the purpose of the new flag, users in such situations would still be able to use np. We could also keep --any-branch but deprecate it / make np warn users with a HUGE WARNING when they use it.

@sindresorhus
Copy link
Owner

I think we should add --branch, but keep --any-branch. --branch would be for when you know ahead of time the branch name. --any-branch would be for when you don't, or you need to quickly override it, or you just want to opt-out and permanently allow any branch.

I think --branch should probably be named --default-branch to be explicit.


As someone who has built a lot of devops tooling for companies, I can tell you that in many cases, custom release branch names are actually derived from the version to be released.

Those users can continue to use --any-branch.


If we go with --branch we could add a wildcard * option

I think keeping --any-branch is better than magic values like that.

@sholladay
Copy link
Contributor

sholladay commented Nov 26, 2019

The idea for a wildcard kind of plays into why I didn't want to have a --branch option that allows custom branch names. Once you have that feature, it seems like it should support an array of values and regex patterns. It's prone to feature creep. All for a simple if statement that throws an error to prevent you from doing something that may or may not be a mistake (i.e. you might just want to release from a different branch than usual).

For people who would use this feature, it's probably to be used in a larger release script. And I think those people should just do git checkout XYZ && np --any-branch. I see no benefit to git checkout XYZ && np --branch=XYZ. It's effectively a no-op.

I could see more of a benefit if the feature actually did the git checkout for you. But maybe that's too magic? And in my experience, most fancy release scripts are doing some other work on the branch before publishing, in which case they'll still need to git checkout XYZ before np, anyway.

All that said, np has a lot of users now. Maybe the flexibility is worth it.

@sindresorhus
Copy link
Owner

Once you have that feature, it seems like it should support an array of values and regex patterns.

We'll just clearly document that for anything else other than setting a static branch name, people should just use --any-branch and do their own logic.

@hewIngram
Copy link

Opened a PR that should address this: #476
Let me know if there are any comments.

@sholladay your point around it spiralling is a really good one (and of it being fairly limited in use). My need for it was specifically around the fact that we use release as the name of all of our default branches rather than master and I didn't want to have to wrap np up in anything in order to be able to use it.

(just read @howardroark original comment on this and it's the same reason 🙌)

@cspotcode
Copy link

I've started hitting this since some of the projects I work on have switched to a main branch instead of master.

@ckknight
Copy link

Yep, I just ran into this, as we have switched away from master to main

@sholladay
Copy link
Contributor

Can't this issue be closed now that PR #558 has been merged? That PR closed issue #466, which seems like a duplicate of this issue.

@hewIngram
Copy link

Can't this issue be closed now that PR #558 has been merged? That PR closed issue #466, which seems like a duplicate of this issue.

Yeah I think so. The feature is documented in the config section of the readme for anyone that needs it.

@cspotcode and @ckknight - theres a branch flag in the config for np that lets you set a default branch other than master

@tommy-mitchell
Copy link
Collaborator

@sindresorhus this issue can be closed as well

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

No branches or pull requests

10 participants