Skip to content

chore: fix version bump for Java #655

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 8, 2022
Merged

chore: fix version bump for Java #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d41e4a9
chore: fix version bump for Java
shortcuts Jun 8, 2022
095ce64
update tests
shortcuts Jun 8, 2022
6be3bf6
review
shortcuts Jun 8, 2022
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
94 changes: 92 additions & 2 deletions scripts/release/__tests__/createReleasePR.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getSkippedCommitsText,
decideReleaseStrategy,
readVersions,
getNextVersion,
} from '../createReleasePR';

describe('createReleasePR', () => {
Expand Down Expand Up @@ -79,16 +80,19 @@ describe('createReleasePR', () => {
javascript: {
current: '0.0.1',
releaseType: 'patch',
next: getNextVersion('0.0.1', 'patch'),
},

php: {
current: '0.0.1',
releaseType: 'patch',
next: getNextVersion('0.0.1', 'patch'),
},

java: {
current: '0.0.1',
releaseType: 'patch',
next: getNextVersion('0.0.1', 'patch'),
},
})
).toMatchInlineSnapshot(`
Expand All @@ -104,6 +108,7 @@ describe('createReleasePR', () => {
javascript: {
current: '0.0.1',
releaseType: 'patch',
next: getNextVersion('0.0.1', 'patch'),
},

php: {
Expand All @@ -115,6 +120,7 @@ describe('createReleasePR', () => {
java: {
current: '0.0.1',
releaseType: 'patch',
next: getNextVersion('0.0.1', 'patch'),
},
})
).toMatchInlineSnapshot(`
Expand All @@ -130,22 +136,25 @@ describe('createReleasePR', () => {
javascript: {
current: '0.0.1',
releaseType: 'patch',
next: getNextVersion('0.0.1', 'patch'),
},

php: {
current: '0.0.1',
releaseType: 'minor',
next: getNextVersion('0.0.1', 'minor'),
},

java: {
current: '0.0.1',
releaseType: 'patch',
releaseType: null,
skipRelease: true,
next: getNextVersion('0.0.1', null),
},
})
).toMatchInlineSnapshot(`
"- javascript: 0.0.1 -> **\`patch\` _(e.g. 0.0.2)_**
- ~java: 0.0.1 -> **\`patch\` _(e.g. 0.0.2)_**~
- ~java: 0.0.1 -> **\`null\` _(e.g. 0.0.1)_**~
- No \`feat\` or \`fix\` commit, thus unchecked by default.
- php: 0.0.1 -> **\`minor\` _(e.g. 0.1.0)_**"
`);
Expand Down Expand Up @@ -178,6 +187,7 @@ describe('createReleasePR', () => {
});

expect(versions.javascript.releaseType).toEqual('major');
expect(versions.javascript.next).toEqual('1.0.0');
});

it('bumps minor version for feat', () => {
Expand Down Expand Up @@ -205,6 +215,7 @@ describe('createReleasePR', () => {
});

expect(versions.php.releaseType).toEqual('minor');
expect(versions.php.next).toEqual('0.1.0');
});

it('bumps patch version for fix', () => {
Expand Down Expand Up @@ -232,6 +243,7 @@ describe('createReleasePR', () => {
});

expect(versions.java.releaseType).toEqual('patch');
expect(versions.java.next).toEqual('0.0.2');
});

it('marks noCommit for languages without any commit', () => {
Expand Down Expand Up @@ -261,6 +273,8 @@ describe('createReleasePR', () => {
expect(versions.javascript.noCommit).toEqual(true);
expect(versions.php.noCommit).toEqual(true);
expect(versions.java.noCommit).toBeUndefined();
expect(versions.java.releaseType).toEqual('patch');
expect(versions.java.next).toEqual('0.0.2');
});

it('releases every languages if a `specs` commit is present', () => {
Expand Down Expand Up @@ -289,10 +303,13 @@ describe('createReleasePR', () => {

expect(versions.javascript.noCommit).toBeUndefined();
expect(versions.javascript.releaseType).toEqual('patch');
expect(versions.javascript.next).toEqual('0.0.2');
expect(versions.php.noCommit).toBeUndefined();
expect(versions.php.releaseType).toEqual('patch');
expect(versions.php.next).toEqual('0.0.2');
expect(versions.java.noCommit).toBeUndefined();
expect(versions.java.releaseType).toEqual('patch');
expect(versions.java.next).toEqual('0.0.2');
});

it('bumps for `specs` feat with only language `fix` commits', () => {
Expand Down Expand Up @@ -328,10 +345,13 @@ describe('createReleasePR', () => {

expect(versions.javascript.noCommit).toBeUndefined();
expect(versions.javascript.releaseType).toEqual('minor');
expect(versions.javascript.next).toEqual('0.1.0');
expect(versions.php.noCommit).toBeUndefined();
expect(versions.php.releaseType).toEqual('minor');
expect(versions.php.next).toEqual('0.1.0');
expect(versions.java.noCommit).toBeUndefined();
expect(versions.java.releaseType).toEqual('minor');
expect(versions.java.next).toEqual('0.1.0');
});

it('marks skipRelease for patch upgrade without fix commit', () => {
Expand Down Expand Up @@ -361,6 +381,76 @@ describe('createReleasePR', () => {
expect(versions.java.skipRelease).toBeUndefined();
expect(versions.php.skipRelease).toBeUndefined();
});

it('consider prerelease version and correctly bumps them', () => {
const versions = decideReleaseStrategy({
versions: {
javascript: {
current: '0.0.1-alpha',
},
java: {
current: '0.0.1-beta',
},
php: {
current: '0.0.1-algolia',
},
},
commits: [
{
hash: 'b2501882',
type: 'feat',
scope: 'specs',
message: 'add some descriptions',
raw: 'b2501882 feat(specs): add some descriptions',
},
],
});

expect(versions.javascript.noCommit).toBeUndefined();
expect(versions.javascript.releaseType).toEqual('prerelease');
expect(versions.javascript.next).toEqual('0.0.1-alpha.0');
expect(versions.php.noCommit).toBeUndefined();
expect(versions.php.releaseType).toEqual('prerelease');
expect(versions.php.next).toEqual('0.0.1-algolia.0');
expect(versions.java.noCommit).toBeUndefined();
expect(versions.java.releaseType).toEqual('prerelease');
expect(versions.java.next).toEqual('0.0.1-beta.0');
});

it('bumps SNAPSHOT versions correctly', () => {
const versions = decideReleaseStrategy({
versions: {
javascript: {
current: '0.0.1-alpha',
},
java: {
current: '0.0.1-SNAPSHOT',
},
php: {
current: '0.0.1-beta',
},
},
commits: [
{
hash: 'b2501882',
type: 'feat',
scope: 'specs',
message: 'add some descriptions',
raw: 'b2501882 feat(specs): add some descriptions',
},
],
});

expect(versions.javascript.noCommit).toBeUndefined();
expect(versions.javascript.releaseType).toEqual('prerelease');
expect(versions.javascript.next).toEqual('0.0.1-alpha.0');
expect(versions.php.noCommit).toBeUndefined();
expect(versions.php.releaseType).toEqual('prerelease');
expect(versions.php.next).toEqual('0.0.1-beta.0');
expect(versions.java.noCommit).toBeUndefined();
expect(versions.java.releaseType).toEqual('minor');
expect(versions.java.next).toEqual('0.1.0-SNAPSHOT');
});
});

describe('getSkippedCommitsText', () => {
Expand Down
50 changes: 46 additions & 4 deletions scripts/release/createReleasePR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export function readVersions(): VersionsWithoutReleaseType {

export function getVersionChangesText(versions: Versions): string {
return LANGUAGES.map((lang) => {
const { current, releaseType, noCommit, skipRelease } = versions[lang];
const { current, releaseType, noCommit, skipRelease, next } =
versions[lang];

if (noCommit) {
return `- ~${lang}: ${current} (${TEXT.noCommit})~`;
Expand All @@ -52,8 +53,6 @@ export function getVersionChangesText(versions: Versions): string {
return `- ~${lang}: (${TEXT.currentVersionNotFound})~`;
}

const next = semver.inc(current, releaseType!);

if (skipRelease) {
return [
`- ~${lang}: ${current} -> **\`${releaseType}\` _(e.g. ${next})_**~`,
Expand Down Expand Up @@ -149,6 +148,38 @@ export function parseCommit(commit: string): Commit {
};
}

/**
* Returns the next version of the client.
*/
export function getNextVersion(
current: string,
releaseType: semver.ReleaseType | null
): string {
if (releaseType === null) {
return current;
}

let nextVersion: string | null = current;

// snapshots should not be bumped as prerelease
if (!current.endsWith('-SNAPSHOT')) {
nextVersion = semver.inc(current, releaseType);
} else {
nextVersion = `${semver.inc(
current.replace('-SNAPSHOT', ''),
releaseType
)}-SNAPSHOT`;
}

if (!nextVersion) {
throw new Error(
`Unable to bump version: '${current}' with release type: '${releaseType}'`
);
}

return nextVersion;
}

/* eslint-disable no-param-reassign */
export function decideReleaseStrategy({
versions,
Expand All @@ -170,15 +201,23 @@ export function decideReleaseStrategy({
...version,
noCommit: true,
releaseType: null,
next: getNextVersion(currentVersion, null),
};
return versionsWithReleaseType;
}

if (semver.prerelease(currentVersion)) {
console.log(`Deciding next version bump for ${lang}.`);

// snapshots should not be bumped as prerelease
if (
semver.prerelease(currentVersion) &&
!currentVersion.endsWith('-SNAPSHOT')
) {
// if version is like 0.1.2-beta.1, it increases to 0.1.2-beta.2, even if there's a breaking change.
versionsWithReleaseType[lang] = {
...version,
releaseType: 'prerelease',
next: getNextVersion(currentVersion, 'prerelease'),
};
return versionsWithReleaseType;
}
Expand All @@ -191,6 +230,7 @@ export function decideReleaseStrategy({
versionsWithReleaseType[lang] = {
...version,
releaseType: 'major',
next: getNextVersion(currentVersion, 'major'),
};
return versionsWithReleaseType;
}
Expand All @@ -200,6 +240,7 @@ export function decideReleaseStrategy({
versionsWithReleaseType[lang] = {
...version,
releaseType: 'minor',
next: getNextVersion(currentVersion, 'minor'),
};
return versionsWithReleaseType;
}
Expand All @@ -208,6 +249,7 @@ export function decideReleaseStrategy({
...version,
releaseType: 'patch',
...(commitTypes.has('fix') ? undefined : { skipRelease: true }),
next: getNextVersion(currentVersion, 'patch'),
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we include next here, while we can compute it anywhere based on currentVersion and releaseType?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops sorry I had auto merge on.

It was previously computed at the changelog creation level, I've put it a level above as this is where the releaseType is computed. Do you think there's a better place for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

A bit of history FYI, I avoided to add next version there especially because JS actually have independent versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've made sure here it still follows the same logic: #656

};
return versionsWithReleaseType;
},
Expand Down
1 change: 1 addition & 0 deletions scripts/release/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { Language } from '../types';
export type Version = {
current: string;
releaseType: ReleaseType | null;
next?: string;
skipRelease?: boolean;
noCommit?: boolean;
};
Expand Down