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

feat: detect if a release is necessary #750

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions command.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ const yargs = require('yargs')
default: defaults.gitTagFallback,
describe: 'fallback to git tags for version, if no meta-information file is found (e.g., package.json)'
})
.option('detect-release', {
type: 'boolean',
default: defaults.detectRelease,
describe: 'Detect if a release is necessary'
})
.option('path', {
type: 'string',
describe: 'Only populate commits made under this path'
Expand Down
1 change: 1 addition & 0 deletions defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const defaults = {
skip: {},
dryRun: false,
gitTagFallback: true,
detectRelease: false,
preset: require.resolve('conventional-changelog-conventionalcommits')
}

Expand Down
2 changes: 1 addition & 1 deletion lib/checkpoint.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ const figures = require('figures')
const util = require('util')

module.exports = function (argv, msg, args, figure) {
const defaultFigure = args.dryRun ? chalk.yellow(figures.tick) : chalk.green(figures.tick)
const defaultFigure = argv.dryRun ? chalk.yellow(figures.tick) : chalk.green(figures.tick)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to highlight this change, args.dryRun seems to have been a typo.
Looking at the usage of args the expected is an array and not an object.

console.info((figure || defaultFigure) + ' ' + util.format.apply(util, [msg].concat(args.map(function (arg) {

if (!argv.silent) {
console.info((figure || defaultFigure) + ' ' + util.format.apply(util, [msg].concat(args.map(function (arg) {
return chalk.bold(arg)
Expand Down
43 changes: 41 additions & 2 deletions lib/lifecycles/bump.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ async function Bump (args, version) {
if (stdout && stdout.trim().length) args.releaseAs = stdout.trim()
const release = await bumpVersion(args.releaseAs, version, args)
if (!args.firstRelease) {
if (args.detectRelease && !release.releaseType) {
checkpoint(args, release.reason, [])
process.exit()
}
const releaseType = getReleaseType(args.prerelease, release.releaseType, version)
newVersion = semver.valid(releaseType) || semver.inc(version, releaseType, args.prerelease)
updateConfigs(args, newVersion)
Expand Down Expand Up @@ -77,7 +81,12 @@ function isInPrerelease (version) {
return Array.isArray(semver.prerelease(version))
}

const TypeList = ['major', 'minor', 'patch'].reverse()
const MAJOR = 'major'
const MINOR = 'minor'
const PATCH = 'patch'
const VERSIONS = [MAJOR, MINOR, PATCH]

const TypeList = [...VERSIONS].reverse()

/**
* extract the in-pre-release type in target version
Expand Down Expand Up @@ -106,6 +115,8 @@ function getTypePriority (type) {
}

function bumpVersion (releaseAs, currentVersion, args) {
const releaseTypes = args.types.reduce((acc, current) => (!current.hidden ? [...acc, current.type] : acc), [])

return new Promise((resolve, reject) => {
if (releaseAs) {
return resolve({
Expand All @@ -120,7 +131,34 @@ function bumpVersion (releaseAs, currentVersion, args) {
debug: args.verbose && console.info.bind(console, 'conventional-recommended-bump'),
preset: presetOptions,
path: args.path,
tagPrefix: args.tagPrefix
tagPrefix: args.tagPrefix,
whatBump: (commits) => {
let level = PATCH
let breakings = 0
let features = 0

commits.forEach(commit => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is most of the way there, but one concern, the conventional-commits strategy has a more complex whatBump implementation:

https://github.com/conventional-changelog/conventional-changelog/blob/8076d4666c2a3ea728b95bf1e4e78d4c7189b1dc/packages/conventional-changelog-conventionalcommits/conventional-recommended-bump.js

Which includes support for ! in the commit body.

Is it possible for us to use whatever default whatBump is configured already, and then update the reason afterwards?

Copy link
Author

@nelsonfncosta nelsonfncosta May 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially, I had that approach in mind but after some exploring, I was not able to find a clear way to get the default whatBump implementation, ending up implementing my custom whatBump :/

💭 Would be cool if conventional-recommended-bump passed the default whatBump callback as an option argument to the whatBump option, whatBump(commits, { defaultWhatBump, ...}) {}; 💭
It sends a bunch of options but none we can use.

How do you think we should proceed @bcoe ?
I can adapt the current implementation to follow the strategy you linked, to support the ! use case you mentioned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nelsonfncosta I think each preset should be exporting its recommendedBump configuration, see:

https://github.com/conventional-changelog/conventional-changelog/blob/8076d4666c2a3ea728b95bf1e4e78d4c7189b1dc/packages/conventional-changelog-conventionalcommits/index.js#L29

The problem with copying the logic over from any one preset, is that we allow folks to configure their own preset -- so although the logic would be good for conventional commits, it would be off potentially for another plugin.

I haven't tried to do this myself, and apologize for how much of a yarn ball the conventional changelog codebase is -- I think though you should be able to pluck whatBump off of a given preset.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will take a closer look into this! 👀
The information we need is on the recommendedBumpOpts, so if this is being exported it should be a straightforward change 👍

Copy link
Author

@nelsonfncosta nelsonfncosta May 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have been looking for a way to get the preset config, they are not directly exported
something along the lines

const config = require('conventional-changelog-conventionalcommits')

const { recommendedBumpOpts } = config(args)

the problem I'm facing I'm not really sure how to "wait/get" my value from here

if (commit.notes.length > 0) {
breakings += commit.notes.length
level = MAJOR
} else if (releaseTypes.includes(commit.type)) {
features += 1

level = level === PATCH ? MINOR : level
}
})

if (breakings + features === 0) {
return {
level: null,
reason: `No commits found for types: [${releaseTypes.join(', ')}], skipping release stage.`
}
}

return {
level: VERSIONS.indexOf(level)
}
}
}, function (err, release) {
if (err) return reject(err)
else return resolve(release)
Expand Down Expand Up @@ -169,3 +207,4 @@ function updateConfigs (args, newVersion) {
}

module.exports = Bump
module.exports.bumpVersion = bumpVersion
86 changes: 86 additions & 0 deletions test/bump.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/* global describe it beforeEach afterEach */

'use strict'

const { bumpVersion } = require('../lib/lifecycles/bump')
const shell = require('shelljs')
const chai = require('chai')
const { expect } = chai

describe('bumpVersion', () => {
const args = {
types: [
{ type: 'feat', section: 'Features' },
{ type: 'test', section: 'Tests', hidden: true }
]
}

beforeEach(() => {
shell.rm('-rf', 'tmp')
shell.config.silent = true
shell.mkdir('tmp')
shell.cd('tmp')
shell.exec('git init')
shell.exec('git config commit.gpgSign false')
shell.exec('git config core.autocrlf false')
})

afterEach(() => {
shell.cd('../')
shell.rm('-rf', 'tmp')
})

describe('when a tag is avaialble', () => {
let result

beforeEach(async () => {
shell.exec('git commit --allow-empty -m "first-commit"')
shell.exec('git tag 1.2.3')
})

describe('and release commits are present', () => {
beforeEach(async () => {
shell.exec('git commit --allow-empty -m "feat: second-commit"')

result = await bumpVersion(null, '1.2.3', args)
})

it('should return a release recommendation', async () => {
expect(result).to.include({ level: 1, releaseType: 'minor' })
})
})

describe('and no release commits are present', () => {
beforeEach(async () => {
shell.exec('git commit --allow-empty -m "test: second-commit"')

result = await bumpVersion(null, '1.2.3', args)
})

it('should return no release', async () => {
expect(result).to.include({
level: null,
reason: 'No commits found for types: [feat], skipping release stage.'
})
})
})
})

describe('when no tag is found', () => {
let result

beforeEach(async () => {
shell.exec('git commit --allow-empty -m "first-commit"')
shell.exec('git commit --allow-empty -m "second-commit"')

result = await bumpVersion(null, '1.2.3', args)
})

it('should return no release', async () => {
expect(result).to.include({
level: null,
reason: 'No commits found for types: [feat], skipping release stage.'
})
})
})
})