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

Choose default targetVersion using Gradle/Maven configuration #1478

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

joelittlejohn
Copy link
Owner

Relates to #1473.

@joelittlejohn joelittlejohn changed the title Improve default value for targetVersion Choose default targetVersion using Gradle/Maven configuration Feb 12, 2023
@joelittlejohn joelittlejohn force-pushed the default-target-version branch 2 times, most recently from 76fdd36 to bb17e82 Compare February 13, 2023 22:38
@joelittlejohn joelittlejohn added this to the 1.2.0 milestone Feb 14, 2023
@joelittlejohn joelittlejohn marked this pull request as ready for review February 14, 2023 19:53
@joelittlejohn joelittlejohn merged commit eea0a87 into master Feb 14, 2023
@joelittlejohn joelittlejohn deleted the default-target-version branch February 14, 2023 20:40
if (!configuration.targetVersion) {
def compileJavaTask = project.getTasksByName("compileJava", false).first()
configuration.targetVersion = compileJavaTask.getProperties().get("sourceCompatibility")
logger.warn 'Using Gradle targetCompatibility as targetVersion for jsonschema2pojo: ' + configuration.targetVersion
Copy link

Choose a reason for hiding this comment

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

@joelittlejohn you have now merged this with the inconsistent log message, it should mention sourceCompatibility here, no?

same problem in GenerateJsonSchemaJavaTask.groovy

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oops, thanks I will fix


void setTargetVersion(JsonSchemaExtension configuration) {
if (!configuration.targetVersion) {
def compileJavaTask = project.getTasksByName("compileJava", false).first()
Copy link
Collaborator

@unkish unkish Feb 17, 2023

Choose a reason for hiding this comment

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

@joelittlejohn Sorry for reviving this PR.
Could this be the cause for actions failure

Execution failed for task ':app:generateJsonSchema2PojoForDebug'.
Cannot access first() element from an empty Iterable

Neither gradle nor android expert, but could it be that something similar to:

  void setTargetVersion(JsonSchemaExtension configuration) {
    if (!configuration.targetVersion) {
      configuration.targetVersion = project.android.compileOptions.sourceCompatibility
      logger.info 'Using Gradle sourceCompatibility as targetVersion for jsonschema2pojo: ' + configuration.targetVersion
    }

should be used (perhaps even in a null safe way?) for android (see Android Gradle plugin API reference: CompileOptions )?

Given of course that android project is configured with com.android.application plugin

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, clearly the way the detection is working here is not right.

I have replicated the problem locally and I guess a combination of using a different property, and some more defensive coding, would be best.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I don't have time to look this evening but can hopefully fix tomorrow (and release 1.2.1).

Copy link
Owner Author

Choose a reason for hiding this comment

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

We also need to get these CI jobs running against HEAD, not the latest release! :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have time to look this evening but can hopefully fix tomorrow (and release 1.2.1).

Whilst error is certainly not a pleasant one, there might be several workarounds (maybe not the nicest ones) eg.: provide target version in configuration, create/register a dummy compileJava task, appy java plugin.

I don't know whether it is a critical/big pain point for android developers or not, if not - perhaps it would make sense to take some time and not rush ?

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

Successfully merging this pull request may close these issues.

3 participants