stephenlacy / bump-regex

bump regex with semver
MIT License
13 stars 10 forks source link

gulp-bump version 3.0.0 removes the build metadata in the version string #20

Closed many20 closed 6 years ago

many20 commented 6 years ago

Version 3.0.0 removes the Build metadata. Calling "bump({ type: 'prerelease' })" 2.9.0 does not do that

"5.3.0-build.280+id.8" -> "5.3.0-build.281"

build metadata: https://semver.org/#spec-item-10

can it be that index.js line 56 should be different? like: parsed = parsed + (prerelease || '') + (metadata || '')

haven't tested this

stephenlacy commented 6 years ago

https://github.com/stevelacy/bump-regex/issues/18

Now... which is correct with semver or are they unrelated?

stephenlacy commented 6 years ago

tagging @abhishekdev

abhishekdev commented 6 years ago

The following would not maintain the build metadata, as its semver.inc() that later processes the parsed version based on its own implementation and removes the metadata. I could not find a metadata identifier for semver, I guess some spec implementation is missing there (maybe a feature enhancement for that project)

-parsed = parsed + (prerelease || '')
+parsed = parsed + (prerelease || '') + (metadata || '')

But before a discussion starts on how to process the version after semver.inc executes...

@many20: I am curious to know your build flow. More specifically:

@stevelacy: I think bump can only set the metadata using direct assignment (i.e. with opts.version); hence maintaining the metadata for other cases came across as a defect to me as bump behaved different than how semver works. Moreover duplicate metadata, as outlined in #18, was clearly an issue. I think you did the right thing by bumping the major version (which signifies the change as a breaking change). Unless there is a decision to increase the scope1 of bump-regex, I think the metadata handling lies in the scope of semver.

1 There could be new option (like opts.keepmetadata (default: false)) to retain the metatdata always as long as a metadata fragment does not already exist.

many20 commented 6 years ago

my build flow: I use a file named version.json with

{
  "version": "5.3.0-build.221+id.8"
}

me gulpfile.js does:

var fs = require('fs');
var bump = require('gulp-bump');

gulp.task('bump-version-json', function (done) {
    var path = './Scripts/app/';
    var fileName = 'version.json';
    fs.chmod(path + fileName, 0777, function (err) {
        if (err) {
            console.log(err);
            done(err);
            return;
        }
        gulp.src([path + fileName])
            .pipe(bump({ type: 'prerelease' }))
            .pipe(gulp.dest(path));
        done();
    }); 
});

I import this version string in my js program per import { version } from './version';

I use the build metadata to show the current feature set of the build. This is only for dev build used and useful to see that my changes are reached the daily nightbuilds. The build number should be incremented and the major, minor, patch and build metadata should be untouched. gulp.bump (bump-regex) 2.9.0 has this behavior, 3.0.0. removes the build metadata.

@abhishekdev: mmh you are right semver.inc() ignores build metadata. That is the correct behavior.

https://semver.org/#spec-item-11 ... Precedence MUST be calculated by separating the version into major, minor, patch and pre-release identifiers in that order (Build metadata does not figure into precedence). ...

my proposed change does not work (parsed = parsed + (prerelease || '') + (metadata || ''))