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

Tweak @GDScript documentation overall #67100

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Oct 8, 2022

  • Made use of [param] more frequently,
  • Link to other classes' documentation more often.
  • Improve and add more examples.
  • Made the writing style closer to how the rest of the documentation is formatted.
  • Ensure all of these are called "functions", not "methods". (methods are associated with objects, functions are not)

Most notably, removed "It must be a static string, so format strings can't be used." from assert(), as this behavior is actually a bug.

@Mickeon Mickeon requested a review from a team as a code owner October 8, 2022 21:31
@Calinou Calinou added this to the 4.0 milestone Oct 8, 2022
@Mickeon Mickeon force-pushed the doc-peeves branch 2 times, most recently from bb1ef16 to 42d2c59 Compare October 9, 2022 09:17
Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

Regarding the code examples these should be also unified to either use print or not.
Either:

print(something) # Prints ...

or

something # Returns ...

modules/gdscript/doc_classes/@GDScript.xml Outdated Show resolved Hide resolved
modules/gdscript/doc_classes/@GDScript.xml Show resolved Hide resolved
modules/gdscript/doc_classes/@GDScript.xml Outdated Show resolved Hide resolved
modules/gdscript/doc_classes/@GDScript.xml Outdated Show resolved Hide resolved
modules/gdscript/doc_classes/@GDScript.xml Outdated Show resolved Hide resolved
modules/gdscript/doc_classes/@GDScript.xml Outdated Show resolved Hide resolved
modules/gdscript/doc_classes/@GDScript.xml Outdated Show resolved Hide resolved
@Mickeon
Copy link
Contributor Author

Mickeon commented Oct 9, 2022

Updated the PR with most of the proposed fixes.

Regarding the code examples these should be also unified to either use print or not.

It seems like this document is not the only one plagued by this minor inconsistency. Not that it is that big of a deal, and both can be understood.

I feel like for most functions I would favour saying "Return" because it makes the code more readable. With "print(), a nested function doesn't look nearly as readable, but it can be better in some cases, such as when showing what a variable ends up equaling to (print(a))

But it could be ideal to clean it up another time. I don't think I've seen an agreed reason to use one or the other.

- Made use of [param] more frequently,
- Link to other classes' documentation more often, improve the examples.
- Made the writing style closer to how the rest of the documentation is formatted.
- Ensure these are called "functions", not "methods".
- Add [b]Warning:[/b] where more appropriate than [b]Note:[/b]

Most notably, removed " It must be a static string, so format strings can't be used.", as this behavior is actually a bug.
@mhilbrunner
Copy link
Member

As with the other one, thanks for contributing to these docs, and thanks to kleonc for the thorough reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants