Skip to content

Enhance javadoc for World#setAutoSave method#12088

Merged
lynxplay merged 3 commits into
PaperMC:mainfrom
NonSwag:autosave-jd-fix
Feb 10, 2025
Merged

Enhance javadoc for World#setAutoSave method#12088
lynxplay merged 3 commits into
PaperMC:mainfrom
NonSwag:autosave-jd-fix

Conversation

@NonSwag
Copy link
Copy Markdown
Contributor

@NonSwag NonSwag commented Feb 9, 2025

The autosave option has been causing confusion for quite some time now
This change clarifies that disabling auto-save does not stop all saving operations.
This addition explicitly mentions that the world will still save on shutdown and explains the intended purpose of the method.

Clarify that disabling auto-save does not stop all saving operations. This addition explicitly mentions that the world will still save on shutdown and explains the intended purpose of the method.
@NonSwag NonSwag requested a review from a team as a code owner February 9, 2025 14:08
@Warriorrrr Warriorrrr added the type: documentation Documentation stuff label Feb 9, 2025
@lynxplay
Copy link
Copy Markdown
Contributor

lynxplay commented Feb 9, 2025

Can you check other br usages? Semi certain we put them in their own new like just for visual clarity in source.

@NonSwag
Copy link
Copy Markdown
Contributor Author

NonSwag commented Feb 9, 2025

Can you check other br usages? Semi certain we put them in their own new like just for visual clarity in source.

It seems to be very mixed
I can use a paragraph tag instead of two breaks

@NoahvdAa
Copy link
Copy Markdown
Member

NoahvdAa commented Feb 9, 2025

Pretty sure paragraph tags are the standard for Java (and recommended in the docs), we do seem to use both though. Probably better to use <p>?

@Warriorrrr Warriorrrr linked an issue Feb 10, 2025 that may be closed by this pull request
@MiniDigger
Copy link
Copy Markdown
Member

ok, hear me out, markdown https://openjdk.org/jeps/467

@Warriorrrr
Copy link
Copy Markdown
Member

Would using @apiNote make sense for this?

@electronicboy
Copy link
Copy Markdown
Member

Probably more @implNote? 🤷

@emilyy-dev
Copy link
Copy Markdown
Member

@implNote is more of a note for API implementors rather than API users, that's what @apiNote is for, (besides we don't handle @implNote tag)

@NonSwag
Copy link
Copy Markdown
Contributor Author

NonSwag commented Feb 10, 2025

apiNote sounds reasonable

Other methods in the class are already using Note so would it be smart to update those docs in a separate PR as well or just leave them be?

@Warriorrrr
Copy link
Copy Markdown
Member

I don't think we wanna do general cleanup prs yet, so just leave them be for now.

@NonSwag
Copy link
Copy Markdown
Contributor Author

NonSwag commented Feb 10, 2025

Alright, so apiNote it is?

@lynxplay
Copy link
Copy Markdown
Contributor

Yes

@lynxplay lynxplay merged commit 3bd69f2 into PaperMC:main Feb 10, 2025
@NonSwag NonSwag deleted the autosave-jd-fix branch February 11, 2025 04:25
Y2Kwastaken pushed a commit to Y2Kwastaken/Paper that referenced this pull request Feb 16, 2025
Clarify that disabling auto-save does not stop all saving operations. This addition explicitly mentions that the world will still save on shutdown and explains the intended purpose of the method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: documentation Documentation stuff

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

Settings worlds to not auto-save still makes them save

7 participants