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

Replace buildDir with layout.buildDirectory and do some other housekeeping #185

Merged
merged 29 commits into from
Nov 16, 2023

Conversation

labkey-susanh
Copy link
Contributor

Rationale

In Gradle 8.3, the use of project.buildDir was deprecated in favor of project.layout.buildDirectory. We make adjustments here to upgrade to the new world order. We also take this opportunity to do a little housecleaning of a good number of stray semicolons that are cluttering the space and to replace almost all remaining usages of task.findTaskByName with task.named.

Related Pull Requests

Changes

  • Issue 49045 - make copying to the modules-api directory a proper task, so it can run even if apiJar does not
  • Eliminate usages of deprecated project.buildDir
  • Update BuildUtils.includeModules to be able to specify full module paths for exclusion, not just directory names

Copy link
Member

@labkey-tchad labkey-tchad left a comment

Choose a reason for hiding this comment

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

Submitting a partial review because some of these path differences seem like they might break things.

Copy link
Member

@labkey-tchad labkey-tchad left a comment

Choose a reason for hiding this comment

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

For places that could work with any of project.layout.buildDirectory.file("file"), BuildUtils.getBuildDirFile(project, "file"), or a String; is there an order of preference to which one to use?

catch (UnknownTaskException ignore) {
return defaultValue
}
}
Copy link
Member

Choose a reason for hiding this comment

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

For Distribution.getArtifactId

    static Optional<TaskProvider> getOptionalTask(Project project, String taskName)
    {
        try {
            return Optional.of(project.tasks.named(taskName))
        }
        catch (UnknownTaskException ignore) {
            return Optional.empty()
        }
    }

@labkey-susanh
Copy link
Contributor Author

For places that could work with any of project.layout.buildDirectory.file("file"), BuildUtils.getBuildDirFile(project, "file"), or a String; is there an order of preference to which one to use?

I tend to prefer the provider version if I know that it will work, simply because it seems to be the way Gradle is heading, but I also don't think it matters much. It annoys me that it's not at all obvious in various places where the string is needed vs. the file vs. the provider. I'm pretty sure the ant builder interface requires strings most of the time, for example.

Copy link
Member

@labkey-tchad labkey-tchad left a comment

Choose a reason for hiding this comment

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

Looks good. I just pushed a change to remove TaskUtils.getFromTaskIfPresent. They aren't actually used and TaskUtils.getOptionalTask can accomplish the same thing.

@labkey-susanh labkey-susanh merged commit 1a8992d into develop Nov 16, 2023
1 check passed
@labkey-susanh labkey-susanh deleted the fb_buildDirBeGone branch November 16, 2023 14:40
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