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

Gradle plugin - @Input and @Internal should not be apply on the same property #9059

Conversation

vgalloy
Copy link
Contributor

@vgalloy vgalloy commented Mar 24, 2021

Motivation:

In Gradle 7.0 apply @input and @internal on property is forbidden.
Before Gradle 7.0 applying both on the same property is confusing. According to the documentation:

  • @input: Attached to a task property to indicate that the property specifies some input value for the task.
  • @internal: Attached to a task property to indicate that the property is not to be taken into account for up-to-date checking

Note: Gradle 7.0 is not release yet 2021-03-24

Step to reproduce

build.gradle.kts

plugins {
  id("org.openapi.generator") version ("5.1.0")
}

with the command line ./gradlew openApiGenerate will cause

Type 'GenerateTask' property 'input' annotated with @Internal should not be also annotated with @Input.
    
    Reason: A property is ignored but also has input annotations.
    
    Possible solutions:
      1. Remove the input annotations.
      2. Remove the @Internal annotation.

@wing328
Copy link
Member

wing328 commented Mar 30, 2021

Thanks for the PR but your commit (as shown in the Commits tab) is not linked to your Github account, which means this PR won't count as your contribution in https://github.com/OpenAPITools/openapi-generator/graphs/contributors.

Let me know if you need help fixing it.

Ref: https://github.com/OpenAPITools/openapi-generator/wiki/FAQ#how-can-i-update-commits-that-are-not-linked-to-my-github-account

Motivation:

In Gradle 7.0 apply @input and @internal on property is forbidden.
Before Gradle 7.0 applying both on the same property is confusing. According to the documentation:

* @input: Attached to a task property to indicate that the property specifies some input value for the task.
* @internal: Attached to a task property to indicate that the property is not to be taken into account for up-to-date checking
@vgalloy vgalloy force-pushed the input-and-internal-should-not-be-apply-on-same-property branch from b938ce1 to 8028ea6 Compare March 30, 2021 10:45
@vgalloy
Copy link
Contributor Author

vgalloy commented Mar 30, 2021

@wing328
Thx for your remarque =) Is it correct now ?

@kleini
Copy link

kleini commented Mar 31, 2021

What more is required to get this fix merged and released? I would like to get rid of warnings in my builds.

@trecloux
Copy link

trecloux commented Apr 8, 2021

I'm also very interested in this fix, how can I help ?

@goerge
Copy link

goerge commented Apr 11, 2021

Gradle 7 has already been released: https://docs.gradle.org/current/release-notes.html
This issue blocks our upgrade. Any chance to get this merged and released as a 5.1.1 soon?
Thanks a lot!

@o-nix
Copy link

o-nix commented Apr 12, 2021

Meh, tried to upgrade to 7.0 and got this error/thread...
So, the checks are green, the commit is associated with the author, what else?

@vinkenfl
Copy link

Meh, tried to upgrade to 7.0 and got this error/thread...
So, the checks are green, the commit is associated with the author, what else?

Same for me. Currently a showstopper for our Gradle 7 migration.

@agilob
Copy link
Contributor

agilob commented Apr 12, 2021

Let's get this in @wing328

@wing328
Copy link
Member

wing328 commented Apr 13, 2021

Thanks for the PR.

Quick question: after this change, users using Gradle 5.x an 6.x can still use the Gradle plug-in without issues, right? In other words, I just want to make sure this change is backward-compatible.

@agilob
Copy link
Contributor

agilob commented Apr 13, 2021

This project where this diff is applied uses: distributionUrl=https\://services.gradle.org/distributions/gradle-5.6.4-all.zip and tests passed, so I guess so but let the author comment on it.

@vgalloy
Copy link
Contributor Author

vgalloy commented Apr 14, 2021

Thanks for the PR.

Quick question: after this change, users using Gradle 5.x an 6.x can still use the Gradle plug-in without issues, right? In other words, I just want to make sure this change is backward-compatible.

Hello @wing328,

Yes, changes include in this PR should be backward compatible. I'm obviously not able to test all Gradle versions from 5.0 to 7.0 but the default behaviour should be backward compatible.

@wing328
Copy link
Member

wing328 commented Apr 14, 2021

@vgalloy thanks for the details. I'm ok with this change.

@iherasymenko
Copy link

@vgalloy thanks for the fix.

@wing328 could you please cut a new release of the plugin with this change included?

@agilob
Copy link
Contributor

agilob commented Apr 18, 2021

@wing328 could you please cut a new release of the plugin with this change included?

There will be a new release in 2 weeks.

@wing328 wing328 added this to the 5.1.1 milestone Apr 30, 2021
@wing328 wing328 changed the title Gradle - @Input and @Internal should not be apply on the same property Gradle plugin - @Input and @Internal should not be apply on the same property Apr 30, 2021
@Shiradit
Copy link

Shiradit commented May 5, 2021

Hey team, any news on the new release?

rm3l added a commit to Cosmo-Tech/cosmotech-api that referenced this pull request May 7, 2021
However, the following issue [1] with the OpenAPI Generator Gradle Plugin
is a current showstopper, preventing us from merging this branch into
'main'.
This has been fixed in [2] and merged upstream, but a new release of
this Gradle Plugin is expected in roughly 2 weeks from now.

[1] OpenAPITools/openapi-generator#9253
[2] OpenAPITools/openapi-generator#9059
@derjust
Copy link

derjust commented May 11, 2021

We just updated our Gradle plugin to 5.1.1 and it seems to fix the issue mentioned here
(Though we still had to manually fix sourceSets & add additional dependencies

@rm3l
Copy link
Contributor

rm3l commented May 11, 2021

While 5.1.1 fixes the issue reported here, there is still an issue preventing us from migrating to Gradle 7 : this time with the openApiValidate task in the Gradle Plugin, as reported in #9328 .
A PR has been submitted in #9453

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet