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

Note added for https://github.com/godot-rust/gdext/pull/365 #109

Merged
merged 8 commits into from
Jul 31, 2023
Merged

Conversation

RefinedDev
Copy link
Contributor

@RefinedDev RefinedDev commented Jul 30, 2023

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

Instead of adding a note to the now-outdated text, could you please adjust the text so that it reflects the status quo? The workaround should no longer be listed. You can still mention that by default, GDExtension runs all Rust classes, but that gdext explicitly changes this behavior.

Also, instead of linking to the PR, you could link to the ExtensionLibrary API docs:
https://godot-rust.github.io/docs/gdext/master/godot/init/trait.ExtensionLibrary.html

Please also use footnote style for links: [Some text][id] and on the bottom [id]: link (see existing files).

src/gdext/intro/hello-world.md Outdated Show resolved Hide resolved
@Bromeon Bromeon added the outdated-api An API change needs to be reflected in the docs label Jul 30, 2023
@RefinedDev
Copy link
Contributor Author

Apologies for not using footnote-style for links, I have edited the 'Always-on' paragraph as per the status quo and changed the warning into a tip.

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks for the update, no need to apologize! 🙂
A few more comments, then should be good to go!

src/gdext/index.md Outdated Show resolved Hide resolved
src/gdext/index.md Outdated Show resolved Hide resolved
RefinedDev and others added 2 commits July 31, 2023 16:34
Co-authored-by: Jan Haller <[email protected]>
Co-authored-by: Jan Haller <[email protected]>
@RefinedDev
Copy link
Contributor Author

Thank you, and sorry for the inconvenience about the grammar. Are we good to go now?

@Bromeon
Copy link
Member

Bromeon commented Jul 31, 2023

Thank you!

@Bromeon Bromeon merged commit df43e74 into godot-rust:master Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
outdated-api An API change needs to be reflected in the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants