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.

Sign up for GitHub

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

Jump to bottom

[1.21.x] perform savedata cleanup on the io thread #2016

Open
wants to merge 1 commit into
base: 1.21.x
Choose a base branch
from
Open

[1.21.x] perform savedata cleanup on the io thread #2016

wants to merge 1 commit into from
+13 −7

Conversation

thiakil
Copy link
Contributor

@thiakil thiakil commented Mar 9, 2025

Move the SavedDate temp file cleanup to the ioPool.

#2015 but with the vanilla worker system

@neoforged-automation neoforged-automation bot added the 1.21.4 Targeted at Minecraft 1.21.4 label Mar 9, 2025
@neoforged-pr-publishing
Copy link

  • Publish PR to GitHub Packages

@neoforged-compatibility-checks
Copy link

@thiakil, this PR introduces breaking changes.
Fortunately, this project is currently accepting breaking changes, but if they are not intentional, please revert them.
Last checked commit: 206c5528a3ac6d2702c4d339ac580e846850715a.

neoforge (:neoforge)

  • net/neoforged/neoforge/common/IOUtilities
    • cleanupTempFiles(Ljava/nio/file/Path;Ljava/lang/String;)V: ❗ API method was removed

ChampionAsh5357
ChampionAsh5357 requested changes Mar 11, 2025
View reviewed changes
Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

Just some thoughts about how the cleanup function is working.

ChampionAsh5357
ChampionAsh5357 approved these changes Mar 11, 2025
View reviewed changes
Copy link
Contributor

@ChampionAsh5357 ChampionAsh5357 left a comment

Choose a reason for hiding this comment

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

I would like one more person to review and also a quick double check on whether we are still accepting breaking changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChampionAsh5357 ChampionAsh5357 ChampionAsh5357 approved these changes

Assignees
No one assigned
Labels
1.21.4 Targeted at Minecraft 1.21.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thiakil @ChampionAsh5357