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

fixing gradle issue #2116

Merged
merged 12 commits into from
Dec 10, 2021
6 changes: 3 additions & 3 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,10 @@
]
},
{
"login": "yogikhan",
ykhandelwal913 marked this conversation as resolved.
Show resolved Hide resolved
"name": "Yogesh Khandlewal",
"login": "ykhandelwal913",
"name": "Yogesh Khandelwal",
"avatar_url": "https://avatars3.githubusercontent.com/u/16071601?v=4",
"profile": "https://github.com/yogikhan",
"profile": "https://github.com/ykhandelwal913",
"contributions": [
"code"
]
Expand Down
81 changes: 51 additions & 30 deletions plugins/gradle/__tests__/gradle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,15 @@ describe("Gradle Plugin", () => {
await hooks.version.promise({ bump: Auto.SEMVER.patch });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradle"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=1.0.1`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
]);
});

Expand All @@ -81,12 +84,15 @@ describe("Gradle Plugin", () => {
await hooks.version.promise({ bump: Auto.SEMVER.major });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradle"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=2.0.0`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
]);
});

Expand All @@ -102,12 +108,15 @@ describe("Gradle Plugin", () => {
await hooks.version.promise({ bump: Auto.SEMVER.major });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradle"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=2.0.0`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
]);
});

Expand All @@ -122,12 +131,15 @@ describe("Gradle Plugin", () => {
await hooks.version.promise({ bump: Auto.SEMVER.minor });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradle"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=1.2.0`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
]);
});

Expand All @@ -142,12 +154,15 @@ describe("Gradle Plugin", () => {
await hooks.version.promise({ bump: Auto.SEMVER.patch });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradle"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=1.0.0`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
`-Prelease.newVersion=1.0.0`
]);
});

Expand Down Expand Up @@ -332,12 +347,15 @@ describe("Gradle Plugin", () => {
await hooks.version.promise({ bump: Auto.SEMVER.patch });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradle"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=1.0.0`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
]);
});
});
Expand Down Expand Up @@ -367,12 +385,15 @@ describe("Gradle Plugin - Custom Command", () => {
await hooks.version.promise({ bump: Auto.SEMVER.patch });

expect(spy).toHaveBeenCalledWith(expect.stringMatching("gradlew"), [
"release",
"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=1.0.1`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
"-P prop=val",
]);
});
Expand Down Expand Up @@ -408,4 +429,4 @@ describe("getProperties", () => {
exec.mockReturnValueOnce("");
expect(await getProperties("", [])).toStrictEqual({});
});
});
});
13 changes: 8 additions & 5 deletions plugins/gradle/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,15 @@ export default class GradleReleasePluginPlugin implements IPlugin {
if (buildFlag) {
// don't create release, tag, or commit since auto will do this
await execPromise(this.options.gradleCommand, [
"release",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This code is the same as the other block in the conditional. At the very least we need to ensure that the buildFlag is respected and the projects buildTasks is called to ensure that anything relying on the version string is re-built.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm thinking that we'll probably also want to ensure that other features of the release plugin are still being utilized here, like checkSnapshotDependencies.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ykhandelwal913 This looks better. The only thing I see here that is a little confusing is the createScmAdapter, initScmAdapter, and unSnapshotVersion task calls.

For both createScmAdapter and initScmAdapter, I can maybe see some issues here if those aren't initialized by some dependent task, but would like to know for sure if you saw an error without them.

For unSnapshotVersion, I think we should try to avoid this one if we can since this would actually change the version in the version file, which we don't want because we are already changing it ourselves with updateVersion.

Does that make sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

createScmAdapter and initScmAdapter are internal dependency to subsequent task. Hence added them. Removed the unSnapshot version

"createScmAdapter",
"initScmAdapter",
"checkCommitNeeded",
"checkUpdateNeeded",
"checkSnapshotDependencies",
"runBuildTasks",
"updateVersion",
"-Prelease.useAutomaticVersion=true",
`-Prelease.newVersion=${version}`,
"-x createReleaseTag",
"-x preTagCommit",
"-x commitNewVersion",
...this.options.gradleOptions,
]);
} else {
Expand Down Expand Up @@ -374,4 +377,4 @@ export default class GradleReleasePluginPlugin implements IPlugin {
]);
});
}
}
}