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
Merged

fixing gradle issue #2116

merged 12 commits into from Dec 10, 2021

Conversation

ykhandelwal913
Copy link
Collaborator

@ykhandelwal913 ykhandelwal913 commented Dec 6, 2021

Release Notes

This release fixes the Gradle plugin (#2608) for Gradle 6.8+.

What Changed

#2068 has more details

Why

Trying to update the gradle auto plugin to run the individual release task instead of release uber task.
Todo:

Change Type

Indicate the type of change your pull request is:

  • patch

🐤 Download canary assets:

auto-linux--canary.2116.25204.gz
auto-macos--canary.2116.25204.gz
auto-win.exe--canary.2116.25204.gz

📦 Published PR as canary version: under canary scope @auto-canary@10.32.4--canary.2116.25204.0

✨ Test out this PR locally via:

npm install @auto-canary/bot-list@10.32.4--canary.2116.25204.0
npm install @auto-canary/auto@10.32.4--canary.2116.25204.0
npm install @auto-canary/core@10.32.4--canary.2116.25204.0
npm install @auto-canary/package-json-utils@10.32.4--canary.2116.25204.0
npm install @auto-canary/all-contributors@10.32.4--canary.2116.25204.0
npm install @auto-canary/brew@10.32.4--canary.2116.25204.0
npm install @auto-canary/chrome@10.32.4--canary.2116.25204.0
npm install @auto-canary/cocoapods@10.32.4--canary.2116.25204.0
npm install @auto-canary/conventional-commits@10.32.4--canary.2116.25204.0
npm install @auto-canary/crates@10.32.4--canary.2116.25204.0
npm install @auto-canary/docker@10.32.4--canary.2116.25204.0
npm install @auto-canary/exec@10.32.4--canary.2116.25204.0
npm install @auto-canary/first-time-contributor@10.32.4--canary.2116.25204.0
npm install @auto-canary/gem@10.32.4--canary.2116.25204.0
npm install @auto-canary/gh-pages@10.32.4--canary.2116.25204.0
npm install @auto-canary/git-tag@10.32.4--canary.2116.25204.0
npm install @auto-canary/gradle@10.32.4--canary.2116.25204.0
npm install @auto-canary/jira@10.32.4--canary.2116.25204.0
npm install @auto-canary/magic-zero@10.32.4--canary.2116.25204.0
npm install @auto-canary/maven@10.32.4--canary.2116.25204.0
npm install @auto-canary/microsoft-teams@10.32.4--canary.2116.25204.0
npm install @auto-canary/npm@10.32.4--canary.2116.25204.0
npm install @auto-canary/omit-commits@10.32.4--canary.2116.25204.0
npm install @auto-canary/omit-release-notes@10.32.4--canary.2116.25204.0
npm install @auto-canary/pr-body-labels@10.32.4--canary.2116.25204.0
npm install @auto-canary/released@10.32.4--canary.2116.25204.0
npm install @auto-canary/s3@10.32.4--canary.2116.25204.0
npm install @auto-canary/sbt@10.32.4--canary.2116.25204.0
npm install @auto-canary/slack@10.32.4--canary.2116.25204.0
npm install @auto-canary/twitter@10.32.4--canary.2116.25204.0
npm install @auto-canary/upload-assets@10.32.4--canary.2116.25204.0
npm install @auto-canary/vscode@10.32.4--canary.2116.25204.0
# or 
yarn add @auto-canary/bot-list@10.32.4--canary.2116.25204.0
yarn add @auto-canary/auto@10.32.4--canary.2116.25204.0
yarn add @auto-canary/core@10.32.4--canary.2116.25204.0
yarn add @auto-canary/package-json-utils@10.32.4--canary.2116.25204.0
yarn add @auto-canary/all-contributors@10.32.4--canary.2116.25204.0
yarn add @auto-canary/brew@10.32.4--canary.2116.25204.0
yarn add @auto-canary/chrome@10.32.4--canary.2116.25204.0
yarn add @auto-canary/cocoapods@10.32.4--canary.2116.25204.0
yarn add @auto-canary/conventional-commits@10.32.4--canary.2116.25204.0
yarn add @auto-canary/crates@10.32.4--canary.2116.25204.0
yarn add @auto-canary/docker@10.32.4--canary.2116.25204.0
yarn add @auto-canary/exec@10.32.4--canary.2116.25204.0
yarn add @auto-canary/first-time-contributor@10.32.4--canary.2116.25204.0
yarn add @auto-canary/gem@10.32.4--canary.2116.25204.0
yarn add @auto-canary/gh-pages@10.32.4--canary.2116.25204.0
yarn add @auto-canary/git-tag@10.32.4--canary.2116.25204.0
yarn add @auto-canary/gradle@10.32.4--canary.2116.25204.0
yarn add @auto-canary/jira@10.32.4--canary.2116.25204.0
yarn add @auto-canary/magic-zero@10.32.4--canary.2116.25204.0
yarn add @auto-canary/maven@10.32.4--canary.2116.25204.0
yarn add @auto-canary/microsoft-teams@10.32.4--canary.2116.25204.0
yarn add @auto-canary/npm@10.32.4--canary.2116.25204.0
yarn add @auto-canary/omit-commits@10.32.4--canary.2116.25204.0
yarn add @auto-canary/omit-release-notes@10.32.4--canary.2116.25204.0
yarn add @auto-canary/pr-body-labels@10.32.4--canary.2116.25204.0
yarn add @auto-canary/released@10.32.4--canary.2116.25204.0
yarn add @auto-canary/s3@10.32.4--canary.2116.25204.0
yarn add @auto-canary/sbt@10.32.4--canary.2116.25204.0
yarn add @auto-canary/slack@10.32.4--canary.2116.25204.0
yarn add @auto-canary/twitter@10.32.4--canary.2116.25204.0
yarn add @auto-canary/upload-assets@10.32.4--canary.2116.25204.0
yarn add @auto-canary/vscode@10.32.4--canary.2116.25204.0

@sumwatshade
Copy link
Collaborator

@ykhandelwal913 can you add details and context to the PR so for future reference we can understand this changeset better?

Checks are failing on CI, @adierkens i assume this is because we don't propagate secrets to forks?

@ykhandelwal913
Copy link
Collaborator Author

@sumwatshade sure let me add the details. I am trying to fix #2068

@ykhandelwal913
Copy link
Collaborator Author

ykhandelwal913 commented Dec 6, 2021

@sumwatshade @adierkens let me know if there is anything i need to do to fix the build.

Copy link
Collaborator

@sugarmanz sugarmanz left a comment

Choose a reason for hiding this comment

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

Just want to make sure this has been tested on the latest versions of Gradle, 6.8, 7.0, etc.

@@ -112,12 +112,9 @@ 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

.all-contributorsrc Show resolved Hide resolved
@adierkens
Copy link
Collaborator

I don't see anything in the circle config that would prevent this from building -- it doesn't look like this PR is being picked up by the auto circleci org, but rather the one from @ykhandelwal913 fork. The only thing that I can think of is the PR is made from main and maybe circle is filtering out by branch name?

@adierkens adierkens added the patch Increment the patch version when merged label Dec 8, 2021
@codecov
Copy link

codecov bot commented Dec 8, 2021

Codecov Report

Merging #2116 (e3fc4bd) into main (956f88a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2116   +/-   ##
=======================================
  Coverage   80.19%   80.19%           
=======================================
  Files          66       66           
  Lines        5413     5413           
  Branches     1263     1263           
=======================================
  Hits         4341     4341           
  Misses        709      709           
  Partials      363      363           
Impacted Files Coverage Δ
plugins/gradle/src/index.ts 76.66% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 956f88a...e3fc4bd. Read the comment docs.

@ykhandelwal913
Copy link
Collaborator Author

I don't see anything in the circle config that would prevent this from building -- it doesn't look like this PR is being picked up by the auto circleci org, but rather the one from @ykhandelwal913 fork. The only thing that I can think of is the PR is made from main and maybe circle is filtering out by branch name?

Fixed the build issue as build was running from my fork instead of intuit org. Once made the changes, things worked.

@sugarmanz
Copy link
Collaborator

@adierkens looks like I don't have perms to merge, but looks good to me!

@ykhandelwal913 could you just write small release notes saying this fixes the plugin for Gradle 6.8+ and link the GitHub ticket? Thanks.

@ykhandelwal913
Copy link
Collaborator Author

@adierkens looks like I don't have perms to merge, but looks good to me!

@ykhandelwal913 could you just write small release notes saying this fixes the plugin for Gradle 6.8+ and link the GitHub ticket? Thanks.

will do

@adierkens
Copy link
Collaborator

🚀 PR was released in v10.32.4 🚀

@adierkens adierkens added the released This issue/pull request has been released. label Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch Increment the patch version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants