search

FabricMC / fabric-loom

Gradle build system plugin used to automate the setup of a minecraft mod development environment.
MIT License
233 stars 201 forks source link

Replace Deprecated .getBuildDir with .getLayout().getBuildDirectory() and make Lazy #978

Closed JT122406 closed 9 months ago

JT122406 commented 10 months ago

image

Juuxel commented 10 months ago

See also #970, which would be simple to do together with this change.

JT122406 commented 10 months ago

Do I need to do something to fix the failed tests or?

modmuss50 commented 10 months ago

The failing tests are either random network failuires, or known flakey tests, so dont worry about them. All of the important ones have passed.

JT122406 commented 10 months ago

Anything else need to be done for this to get merged?

modmuss50 commented 10 months ago

Anything else need to be done for this to get merged?

Are you able to make it lazy as suggested in #970 ?

JT122406 commented 10 months ago

Anything else need to be done for this to get merged?

Are you able to make it lazy as suggested in #970 ?

I can give it a shot, Wasn't sure if you wanted that in this or separate pr

modmuss50 commented 10 months ago

Anything else need to be done for this to get merged?

Are you able to make it lazy as suggested in #970 ?

I can give it a shot, Wasn't sure if you wanted that in this or separate pr

Let me know if you have any issues, I think it makes sense to do as part of this PR as it changes the same line of code.

modmuss50 commented 9 months ago

Can you update this PR to make these use a lazy provider? I dont seem to be able to push the change to your branch. Should be good to merge once done.

JT122406 commented 9 months ago

Can you update this PR to make these use a lazy provider? I dont seem to be able to push the change to your branch. Should be good to merge once done.

You should be able to push to my branch

modmuss50 commented 9 months ago

Can you update this PR to make these use a lazy provider? I dont seem to be able to push the change to your branch. Should be good to merge once done.

You should be able to push to my branch

I tried, for some reason it wouldnt allow me to. Maybe due to the fact its the default branch in your repo I'm not sure.

JT122406 commented 9 months ago

Should be good now

JT122406 commented 9 months ago

I'm assuming we don't want lazy in LoomFilesProjectImpl.java

modmuss50 commented 9 months ago

I'm assuming we don't want lazy in LoomFilesProjectImpl.java

No, this looks good now. Thanks. I hope it builds, as I think I had to remove the File import as its no longer required.

JT122406 commented 9 months ago

I'm assuming we don't want lazy in LoomFilesProjectImpl.java

No, this looks good now. Thanks. I hope it builds, as I think I had to remove the File import as its no longer required.

Just handled that

JT122406 commented 9 months ago

Sorry about the wait with this pr kinda forgot about it with everything going on with finals, Thanks for reminding me @modmuss50

modmuss50 commented 9 months ago

No worries at all, thanks a lot. I've also been busy with other things and have only just got around to looking at loom things again 👍