search

SimonAlling / userscripter

Create userscripts in a breeze!
https://www.npmjs.com/package/userscripter
MIT License
100 stars 10 forks source link

feat: Make date suffix in version optional #38

Closed avallete closed 3 years ago

avallete commented 3 years ago

The constant overriding of version with dateAsSemver when in development can cause trouble when trying to create and e2e test the userscript using webextension (ex: chrome refuse to load a webextension with a version is not formated like x.x.x).

Even if we can build using USERSCRIPTER_MODE=production to fix the version issue. But that also minify the code, making the debuging/e2e tests writing harder.

So I simply added an option into the config to opt-out of this default behavior if needed.

SimonAlling commented 3 years ago

Thanks for a great PR! I've never really been satisfied with how that works, and I like your solution. Maybe the name of the option could be clarified so it's obvious that the date part will only be added in nightly and dev builds. appendDateToVersionInNightlyAndDev is a bit long though … :sweat_smile:

avallete commented 3 years ago

I agree, maybe keeping the same option name, but putting it into a sub-object would make it clearer, something like:

{
    devOptions: {
        dateAsSemverVersion
    }
}
SimonAlling commented 3 years ago

Alright, @avallete – I've pushed a commit that renames the option. There are breaking changes on master since the last release, so the next release has to be a major one, and I have at least one more planned feature I want to include in it. But I'm gonna try to backport this feature and release v1.4.0 as soon as the PR has been merged, so you don't have to wait for the next major.

SimonAlling commented 3 years ago

I also rephrased the PR title. We haven't been doing semantic commit messages up until now, but why not seize the opportunity? :slightly_smiling_face:

SimonAlling commented 3 years ago

I found an even better solution that I think makes it more clear what the final version will be. I think the PR is ready for merging; let me know if you agree!

avallete commented 3 years ago

I found an even better solution that I think makes it more clear what the final version will be. I think the PR is ready for merging; let me know if you agree!

Looks great to me, making the option way more explicit indeed.

SimonAlling commented 3 years ago

Userscripter 1.4.0 is now published. :rocket: Thanks for suggesting this improvement, @avallete! :smiley: