search

bchr02 / node-pre-gyp-github

A node-pre-gyp module which provides the ability to publish to GitHub releases.
MIT License
52 stars 32 forks source link

custom version tag #10

Closed royaltm closed 8 years ago

royaltm commented 8 years ago

First of all, nice work - saved me some time.

The issue for me however is that I was generating tag names with "v" prefix, e.g. "v1.0.0".

Currently your script uses hardcoded this.package_json.version as a release tag name, but it would be great if one could simple pass the version tag name via arguments to bin/node-pre-gyp-github.

BTW. I am using access token with only "notifications, public_repo, repo_deployment" access flags enabled and it works fine. Probably "notifications" are not even needed, but can not confirm that yet.

bchr02 commented 8 years ago

@royal thanks.

Yes, this is a good suggestion. I will add it.

And thanks for the info on the access you are using.

On May 20, 2016, at 10:35 PM, royal notifications@github.com wrote:

First of all, nice work - saved me some time.

The issue for me however is that I was generating tag names with "v" prefix, e.g. "v1.0.0".

Currently your script uses hardcoded this.package_json.version as a release tag name, but it would be great if one could simple pass the version tag name via arguments to bin/node-pre-gyp-github.

BTW. I am using access token with only "notifications, public_repo, repo_deployment" access flags enabled and it works fine. Probably "notifications" are not even needed, but can not confirm that yet.

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub

royaltm commented 8 years ago

I've created a pull request. You might not like it due to the breaking change. However I think you'll might find interesting the idea about {version}.

bchr02 commented 8 years ago

@royal can you change it so it's not a breaking change? I think it could be done.

On May 21, 2016, at 12:43 PM, royal notifications@github.com wrote:

I've created a pull request. You might not like it due to the breaking change. However I think you'll might find interesting the idea about {version}.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub

royaltm commented 8 years ago

In order to do that i would have changed uploadAssets() - instead of flat scan of the directory (stage_dir / tag_name) to it would need to deep scan the whole stage_dir until the file is found. Alternatively it could test for (stage_dir / tag_name) first and fall back to (stage_dir). Maybe I'll try the second approach.

royaltm commented 8 years ago

done

bchr02 commented 8 years ago

@royaltm Please see my code comments inline: https://github.com/bchr02/node-pre-gyp-github/pull/11/files#diff-168726dbe96b3ce427e7fedce31bb0bc thanks.

bchr02 commented 8 years ago

I just deleted my previous comment about needing to update line 63. It was a mistake.

royaltm commented 8 years ago

Please see my answers to your comments.

bchr02 commented 8 years ago

Done with: https://github.com/bchr02/node-pre-gyp-github/pull/12

Instead of customizing tag via an option we implemented via a new package.json's binary.remote_path property.

Thank you @royaltm for this.