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

Don't throw an error if version doesn't start with `v´ #743

Closed
leo opened this issue Sep 11, 2016 · 5 comments · May be fixed by qcif/data-curator#563
Closed

Don't throw an error if version doesn't start with `v´ #743

leo opened this issue Sep 11, 2016 · 5 comments · May be fixed by qcif/data-curator#563

Comments

@leo
Copy link

leo commented Sep 11, 2016

IMHO, there's no reason to strictly throw an error here. Instead of doing that, we should just log the suggestion and let things like Travis CI simply pass on with other tasks.

For example: At HyperTerm we're not using the "v" (because there's simply no point in using it), but we would still like to use the "GitHub publisher" feature... 😊

leo added a commit to vercel/hyper that referenced this issue Sep 11, 2016
@develar
Copy link
Member

develar commented Sep 11, 2016

Problem is that we search version by v-prefixed tag name — https://github.com/electron-userland/electron-builder/blob/master/src/publish/gitHubPublisher.ts#L53

Solution — use both variants to find and log warn instead of throwing error.

@leo
Copy link
Author

leo commented Sep 11, 2016

@develar That sounds like a great way to handle it!

@leo
Copy link
Author

leo commented Sep 13, 2016

@develar Is this a big change? Probably not, right? Is there are chance that you could fix this and release a patch? We're going to release a big thing and would like to use this feature for it.

@develar
Copy link
Member

develar commented Sep 18, 2016

@leo Sorry, I missed your last message. Fixed.

build.publish.vPrefixedTagName can be set to false to create tag name without v prefix. Regardless of this option, v is not required anymore, both variants are checked.

@leo
Copy link
Author

leo commented Sep 18, 2016

@develar Cool, thanks! 👍 😊

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

Successfully merging a pull request may close this issue.

2 participants