Reduce change-version.js's scope of modifications

[julien-deramond]

This PR is a suggestion to reduce the build/change-version.js scope of modifications after having seen those issues while releasing Boosted:

• Only modify Bootstrap version in package*.json
• Don't modify versions in site/content/docs files

Let's go back a few days with the commit before "Release v5.1.1":

git checkout b6855ae13817f516f63bd2916ead5243e07f210b
npm i
npm run release-version 5.1.0 5.1.1

From here:

• package.json and package-lock.json are modified but "bootstrap" package version isn't the only one to be modified:

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: package.json
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ package.json:4 @
{
  "name": "bootstrap",
  "description": "The most popular front-end framework for developing responsive, mobile first projects on the web.",
  "version": "5.1.0",
  "version": "5.1.1",
  "config": {
    "version_short": "5.1"
  },
@ package.json:148 @
    "shelljs": "^0.8.4",
    "stylelint": "^13.13.1",
    "stylelint-config-twbs-bootstrap": "^2.2.3",
    "terser": "5.1.0",
    "terser": "5.1.1",
    "vnu-jar": "21.9.2"
  },
  "files": [

so if the release manager doesn't see it, some packages will be updated

• site/content/docs/5.1/**/*.md are modified (4 files are concerned when bumping from 5.1.0 to 5.1.1). This is the type of modifications that we can observe:

─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
modified: site/content/docs/5.1/customize/color.md
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
@ color.md:112 @ Here's how you can use these in your Sass:

## Generating utilities

<small class="d-inline-flex px-2 py-1 font-monospace text-muted border rounded-3">Added in v5.1.0</small>
<small class="d-inline-flex px-2 py-1 font-monospace text-muted border rounded-3">Added in v5.1.1</small>

Bootstrap doesn't include `color` and `background-color` utilities for every color variable, but you can generate these yourself with our [utility API]({{< docsref "/utilities/api" >}}) and our extended Sass maps added in v5.1.0.
Bootstrap doesn't include `color` and `background-color` utilities for every color variable, but you can generate these yourself with our [utility API]({{< docsref "/utilities/api" >}}) and our extended Sass maps added in v5.1.1.

1. To start, make sure you've imported our functions, variables, mixins, and utilities.
2. Use our `map-merge-multiple()` function to quickly merge multiple Sass maps together in a new map.

But as well, those are unwanted updates because those versions must remain the same. I don't know well the history of the project but this type of information ("added in v5.x.x") is relatively new.

[XhmikosR]

Thanks for the PR but some of these changes are intended. Note that our scripts aren't supposed to work with other projects.

Regardless, I'll have a look in the next days but I can't say for sure if we'll land this patch.

[julien-deramond]

There was apparently an issue with parenthesis fixed in 81db8d1be0ba307b05a732099d4367c9cdb32fa7. Without this modification the version was modified in undefined in the files. If you find it interesting at some point, I'll fix the "Lint" issue.

[XhmikosR]

@julien-deramond thanks for your patience :)

I tried this and I have two comments:

1. with your patch, there's one case that's missed: https://github.com/twbs/bootstrap/commit/1eed9cea7f9d6060fafa0cb9e2b8642cd8f89d86 https://github.com/twbs/bootstrap/blob/a5483a8a96eaa88b4da7e432e0b925d0f3943dc3/package-lock.json#L9
2. I think we could also skip any **/dist/** files since those are built anyway

WDYT?

[julien-deramond]

1. with your patch, there's one case that's missed: [1eed9ce](https://github.com/twbs/bootstrap/commit/1eed9cea7f9d6060fafa0cb9e2b8642cd8f89d86) https://github.com/twbs/bootstrap/blob/a5483a8a96eaa88b4da7e432e0b925d0f3943dc3/package-lock.json#L9

Yes indeed. Probably because we were still using "lockfileVersion": 1 at the time that contained only one time the version. Or maybe just a mistake :innocent: Anyway I've modified the replaceRecursively function in order to accept an integer argument to control the max number of replacements/modifications (1 for package.json and 2 for package-lock.json). Let me know if you think there's another way (more elegant) to do that.

2. I think we could also skip any `**/dist/**` files since those are built anyway

Good point! As you said, they will be generated with the npm run release so we don't need to modify them here. The regex pattern is modified accordingly: |(.*\/)?dist\/)

[mdo]

Deferring to you on when to merge this one @XhmikosR :).

[GeoSot]

Supposing I am not missing something, why do we have to replace the version of css and package.lock files? As theses files are the result of another process, could't 'just' do :

• npm run release-version
• npm install
• npm run dist

[XhmikosR]

Yeah, but we don't do npm install after the release script.

[GeoSot]

Yeah, but we don't do npm install after the release script.

Agreeing with this, however is not wise to mess with package.lock . So can we do something else to avoid blind replacement there? 😕

[XhmikosR]

I guess we could run npm version 1.2.3.4 --git-tag-version=false but it's an extra command so it needs to be automated.

[julien-deramond]

We could do something like:

"prerelease-version": "npm version 5.2.0 --git-tag-version=false",
"release-version": "node build/change-version.js",

See Pre & Post Scripts

But I don't know if it is possible to replace 5.2.0 by the second argument when we execute npm run release-version 5.1.3 5.2.0. Something like prerelease-version": "npm version argv[2] --git-tag-version=false" (not working but you see what I mean).

[GeoSot]

I guess we could run npm version 1.2.3.4 --git-tag-version=false but it's an extra command so it needs to be automated.

or something like this?

const exec = require('child_process').execSync
const pkg = require('../package.json')

const existingVersion = pkg.config.version

const [versionToGo] = process.argv.slice(2) // read cli argument

exec(`npm version ${versionToGo} --git-tag-version=false`)
exec('npm install') // handle package.lock
exec('our code making replaces')
exec('npm run dist')
exec('npm run release')

[XhmikosR]

I don't like npm install; it's useless here...

[GeoSot]

I don't like npm install; it's useless here...

I used it to fix the version tag in package-lock, the npm way :wink:

[mdo]

What's the status here? Okay to merge, or needs more changes?

[XhmikosR]

Needs a closer look for sure. I wouldn't hurry.