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

Alternative tag format for prebuild uploads #196

Closed
daviwil opened this issue Jan 19, 2018 · 14 comments
Closed

Alternative tag format for prebuild uploads #196

daviwil opened this issue Jan 19, 2018 · 14 comments

Comments

@daviwil
Copy link
Contributor

daviwil commented Jan 19, 2018

Hey folks! Quick question: what do you think about having a parameter that allows the user to define an alternative format for their release tags when prebuilds get uploaded to GitHub. This would allow projects that don't use the v1.0.0 tag format with the v prefix to continue tagging in their own way while uploading prebuilds. An example of this is node-pty which doesn't add the v prefix to their tags.

Would it be enough to allow the specification of a --tag-prefix so that someone could clear out the v prefix if they needed to, or should we go for a full --tag-format which allows an arbitrary format string, maybe something like "v{VERSION}"?

Looks like the code that controls this is here. It'd need to be updated in prebuild-install too. If you think it's a good idea I'll be happy to send PRs to both projects to enable this.

Thanks!

@ralphtheninja
Copy link
Member

@daviwil --tag-format sounds good, which could default to v${version}.

I'm curious, what's your process when releasing node-pty at the moment? Are you creating the github releases manually? How are you bumping the module version when publishing to npm etc?

@daviwil
Copy link
Contributor Author

daviwil commented Jan 19, 2018

That's a great question, I'd have to ask @Tyriar about that. I forked node-pty to make a node-pty-prebuilt package using prebuild, so I let prebuild create the tag/release after I set the new version in package.json. We're investigating how to migrate the prebuild support back into node-pty itself so that's why I'm looking for a way to accommodate it's existing tagging scheme.

@ralphtheninja
Copy link
Member

@daviwil I'm sure you have your reasons for picking tags without the v-prefix, but have you considered starting using it? 😀 It would mean less complexity for this module, but might be difficult for you to change. I don't really mind the change, since it's not a huge change of complexity, just bringing it up.

@daviwil
Copy link
Contributor Author

daviwil commented Jan 19, 2018

I think node-pty used to have a v prefix in the past but it got dropped at some point. Might just be a matter of taste, I'm not sure :)

@Tyriar
Copy link

Tyriar commented Jan 19, 2018

@ralphtheninja typically for node-pty I do the following:

  1. Update package.json version, commit and push
  2. npm publish
  3. Create the GH release manually.

I never included the v myself, I think it's fairly common not to. VS Code doesn't either for example.

@Tyriar
Copy link

Tyriar commented Jan 19, 2018

Might just be a matter of taste

Pretty much https://github.com/Tyriar/node-pty/tags?after=0.4.1, that's the first release after forking.

@ralphtheninja
Copy link
Member

I personally use npm version patch | minor | major, which bumps package.json for me, commits and tags and that's the reason the v end up there by default.

For leveldown we do:

  1. npm version patch | minor | major
  2. git push

This will trigger prebuild-ci on travis and appveyor, which calls prebuild which creates the release on github and uploads the binaries.

After the binaries have been built, then we do npm publish

@mathiask88
Copy link
Member

I don't mind having a custom tag format, but how do you want to get prebuild-install to know what tag format it should use to search for a release?

@ralphtheninja
Copy link
Member

Sorry if this turned into some meta discussion 😁 We can definitely add the format thingie.

@mathiask88
Copy link
Member

In the moment of writing this I answered the question myself
You could add it to the modules package json :D

  "scripts": {
    "install": "prebuild-install --tag-format xyz || node-gyp rebuild"
  }

@daviwil
Copy link
Contributor Author

daviwil commented Jan 19, 2018

@mathiask88 yep, that's what I had in mind :)

@Tyriar
Copy link

Tyriar commented Jan 19, 2018

It would mean less complexity for this module, but might be difficult for you to change.

Also given this discussion I'm fine with changing to a v prefix considering npm recommends it with npm version, if you don't think this will be generally useful. It'll look a little ugly when looking at tags but node-pty's releases already do as I changed it before.

@daviwil
Copy link
Contributor Author

daviwil commented Jan 19, 2018

It'd definitely be simpler to follow the v convention. I would guess that most people would be following that convention so maybe a --tag-format param isn't as useful to the general public. Probably not a big code change at any rate.

@daviwil
Copy link
Contributor Author

daviwil commented Jan 20, 2018

I think I'll close this for now since node-pty might use the v tag prefix. Thanks for the helpful discussion!

@daviwil daviwil closed this as completed Jan 20, 2018
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

4 participants