Skip to content

Use outputFileProperty instead of outputFile #3460

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

Merged
merged 1 commit into from
Oct 5, 2022

Conversation

ilgonmic
Copy link
Contributor

Because of changes in API of compile task

@qwwdfsad qwwdfsad self-requested a review September 27, 2022 11:24
Copy link
Collaborator

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

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

It does not build that implies breaking changes in the plugin.

Please elaborate on why these changes are necessary in the first place and whether the breaking changes are intended

@ilgonmic
Copy link
Contributor Author

Changes with migration from outputFile to outputDirectory in Gradle plugin are necessary, because can be situation, when users want to use complex module name e.g. (@foo/bar). And for these one outputFile migrated to 2 properties outputDir and moduleName.
Apart from that, outputFile was deprecated earlier with migration plan to outputFileProperty (because of Gradle property API), so there is a plan to leve only property-based inputs and outputs

@qwwdfsad
Copy link
Collaborator

Thanks, but why is it non-buildable then?

I find it concerning that we have a deprecated API that has no replacement until Kotlin 1.8.0, where the original API is dropped

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Oct 3, 2022

@ilgonmic can you please provide an update regarding this one?

@ilgonmic
Copy link
Contributor Author

ilgonmic commented Oct 3, 2022

Yes, sorry

It has a replacement. outputFileProperty is the replacement, using Gradle Property API.
And it existed during deprecation cycle of outputFile

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Oct 3, 2022

I see, thanks. Please target the PR into develop branch and let release team rebase on it in such case

@ilgonmic ilgonmic changed the base branch from kotlinx.train-1.8.20 to develop October 4, 2022 09:39
@ilgonmic ilgonmic changed the base branch from develop to kotlinx.train-1.8.20 October 4, 2022 09:39
@ilgonmic ilgonmic force-pushed the ilgonmic/kotlinx.train-1.8.20 branch from 2d90dd4 to 722422e Compare October 4, 2022 09:58
@ilgonmic ilgonmic changed the base branch from kotlinx.train-1.8.20 to develop October 4, 2022 09:58
@ilgonmic
Copy link
Contributor Author

ilgonmic commented Oct 4, 2022

Done

@qwwdfsad qwwdfsad merged commit 935faf7 into develop Oct 5, 2022
@qwwdfsad qwwdfsad deleted the ilgonmic/kotlinx.train-1.8.20 branch October 5, 2022 09:01
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