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

Plugin Rewrite for Gradle 6+ #91

Merged
merged 24 commits into from
Jun 8, 2021
Merged

Conversation

Azurelol
Copy link
Contributor

@Azurelol Azurelol commented May 31, 2021

Description

Redesigns the plugin for newer version of Gradle; attempts to simplify the architecture.
To be compatible with newer gradle versions we decided to refactor the whole plugin.
The old architecture of the plugin was quite complex to support batchmode calls from the extension.
This feature is gone as it wasn't really used or had any benifits. The support for unity package creation
is also gone since building and releasing them is no longer a prio.

Changes

  • IMPROVE plugin structure
  • BREAK task API
  • REMOVE unity package creation support

@Azurelol Azurelol requested a review from Larusso as a code owner May 31, 2021 08:42
@ghost ghost changed the title Refactor/plugin redesign 1 plugin redesign 1 May 31, 2021
@Azurelol Azurelol changed the base branch from master to release/2.x May 31, 2021 08:43
@ghost ghost removed add fix labels May 31, 2021
@Azurelol Azurelol force-pushed the refactor/plugin_redesign_1 branch from ab09c8e to 1cef5fb Compare May 31, 2021 08:44
Update project to Gradle 6.8.3
Rewrite the API's internals
@Azurelol Azurelol force-pushed the refactor/plugin_redesign_1 branch from 1cef5fb to 7f5776b Compare May 31, 2021 09:38
Copy link
Member

@Larusso Larusso left a comment

Choose a reason for hiding this comment

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

I had a short look and found two issues from my setup (macOS)

I would also like to see that all *IntergrationTest classes are renamed back to *IntegrationSpec. I see these tests more like specifications than plain test files.

import wooga.gradle.unity.UnityTaskIntegrationTest

class ReturnLicenseTaskIntegrationTest extends UnityTaskIntegrationTest<ReturnLicense> {

Copy link
Member

Choose a reason for hiding this comment

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

I needed to add these lines since I don't have an active Unity license. So we must trick the system into thinking we have one.

    def setup() {
        //setup fake license dir so we don't delete actual licenses
        def licenseDir = File.createTempDir("unity","testLicenseDir")
        createFile("testLicense", licenseDir)
        buildFile << """
        unity {
            licenseDirectory.set(new File("${escapedPath(licenseDir.path)}"))
        }
        """.stripIndent()
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 212 to 213
"" | true | "${testTaskName}.log"
"" | false | "${testTaskName}.log"
Copy link
Member

Choose a reason for hiding this comment

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

These two examples error on macOS because the logfile path it prodocues is /${testTaskName}.log which is not writeable. I couldn't find the logic where the full path gets combined.

@Azurelol Azurelol changed the title plugin redesign 1 Plugin Rewrite for Gradle 6+ Jun 1, 2021
@Azurelol Azurelol requested a review from Larusso June 1, 2021 09:46
@Azurelol Azurelol force-pushed the refactor/plugin_redesign_1 branch from 8b3b29a to ca7a1ef Compare June 3, 2021 09:38
Wrap access of project.properties in another provider.
@Larusso Larusso force-pushed the refactor/plugin_redesign_1 branch from 3d55139 to abb4d3c Compare June 4, 2021 09:50
@Larusso Larusso merged commit 6f71b54 into release/2.x Jun 8, 2021
@Larusso Larusso deleted the refactor/plugin_redesign_1 branch June 8, 2021 09:38
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.

2 participants