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

Remove sorting dependency source folders #4334

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

hfhbd
Copy link
Collaborator

@hfhbd hfhbd commented Jul 5, 2023

What is the reason to sort the files?

After digging, it is only used for testing.

@JakeWharton
Copy link
Member

I think this is for determinism with Gradle. Someone commented recently that a non-determism was fixed with 2.0.

@hfhbd
Copy link
Collaborator Author

hfhbd commented Jul 5, 2023

I don't get it, do you have more details?
Gradle should support Set as property type. If Set causes some problems, we should fix this in the gradle plugin.

Background of this PR: I want to fix #3952 with native Gradle dependencies (and configurations) and the list prevents this/makes the implementation more complicated.

@AlecKazakova
Copy link
Collaborator

yep, what jake said

#1548 (comment)

@hfhbd
Copy link
Collaborator Author

hfhbd commented Jul 5, 2023

Hm, the linked commit and issue is about the generated Kotlin code: 4723e6f I just talk about passing the files to SqlCoreEnvironment.

@AlecKazakova
Copy link
Collaborator

aight, gonna leave to after 2.0 since it is a binary incompatible change for the gradle tooling api

@AlecKazakova AlecKazakova added this to the 2.1 milestone Jul 6, 2023
@hfhbd
Copy link
Collaborator Author

hfhbd commented Jul 6, 2023

We don't accept breaking api changes from 2.0.0-rc02 to 2.0.0 but from 2.0.0 to 2.1.0?

@AlecKazakova
Copy link
Collaborator

as long as its not the runtime api im fine

@AlecKazakova
Copy link
Collaborator

only bug fixes from now on, i need to get this release out

@hfhbd
Copy link
Collaborator Author

hfhbd commented Dec 12, 2023

Any plans to review/merge it?

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.

None yet

3 participants