Skip to content

Conversation

@KeithYokoma
Copy link
Contributor

@KeithYokoma KeithYokoma commented Apr 13, 2021

Starting from Gradle 7.0, we need to explicitly annotate the properties with @Input, @Output or @Internal. Also only one getter method is allowed for a property whereas Groovy generates 2 getters (isXXX() and getXXX()) for boolean property.

This pull request adds annotations (LoginTask and UploadArtifactTask properties are not for inputs or outputs so I annotated with @Internal) and suppress generating getXXX() for boolean property by having isXXX() method explicitly.

see also:

  1. Missing annotation: https://docs.gradle.org/7.0/userguide/validation_problems.html#missing_annotation
  2. Redundant getters: https://docs.gradle.org/7.0/userguide/validation_problems.html#redundant_getters

@KeithYokoma KeithYokoma self-assigned this Apr 13, 2021
Copy link
Contributor

@jmatsu jmatsu left a comment

Choose a reason for hiding this comment

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

Good catch! When did the error happen by the way? I think we can cover the case through GradleCompatAcceptanceSpec by adding Gradle 7.0 to the seeds if it's when applying the plugin or running any task (i.e. evaluation at the runtime),


CountDownLatch latch
@Internal CountDownLatch latch
boolean saved
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we need Internal here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have a private getter method below so Gradle considers save is @Internal 👍 (If we have @Internal for save with the private getter method, Gradle complains the redundant annotation).

Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha! 💯

[System.getenv(DeployGatePlugin.ENV_NAME_API_TOKEN)].any()
}

// From Gradle 7.0, only one method to get boolean property value is allowed
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

@KeithYokoma
Copy link
Contributor Author

The error raised when executing loginDeployGate task. I will add the Gradle 7.0 seed in GradleCompatAcceptanceSpec 👍

@KeithYokoma KeithYokoma added this to the 2.4.0 milestone Apr 13, 2021
@KeithYokoma KeithYokoma requested a review from jmatsu April 13, 2021 05:47
Copy link
Contributor

@jmatsu jmatsu left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.


CountDownLatch latch
@Internal CountDownLatch latch
boolean saved
Copy link
Contributor

Choose a reason for hiding this comment

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

Gotcha! 💯

@jmatsu jmatsu merged commit 39458ef into DeployGate:master Apr 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants