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

Fix example for Object._set documentation #80475

Merged
merged 1 commit into from
Aug 25, 2023

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Aug 10, 2023

The method is supposed to return true if the property was set (interestingly the C# example is correct).
Though I wonder if the example shouldn't be improved; it's technically incomplete, because a user might not know what to put instead of print.

@KoBeWi KoBeWi added this to the 4.2 milestone Aug 10, 2023
@KoBeWi KoBeWi requested a review from a team as a code owner August 10, 2023 06:47
@akien-mga
Copy link
Member

Maybe showing a real example of a fake property that actually does something useful would be good indeed.

doc/classes/Object.xml Outdated Show resolved Hide resolved
@akien-mga akien-mga requested a review from a team August 10, 2023 09:36
@akien-mga akien-mga changed the title Fix _set example Fix example for Object._set documentation Aug 10, 2023
doc/classes/Object.xml Outdated Show resolved Hide resolved
doc/classes/Object.xml Outdated Show resolved Hide resolved
@KoBeWi KoBeWi force-pushed the that's_not_how_you_do_set branch 2 times, most recently from a496b85 to 80904f9 Compare August 10, 2023 09:51
doc/classes/Object.xml Outdated Show resolved Hide resolved
@YuriSizov
Copy link
Contributor

If we add a more practical example, prints are probably not needed anymore. Can be kept as code comments instead (i.e. replace prints with # Storing the value in the fake property. or something).

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

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

Looks fine. My suggestion was addressed.

Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

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

The C# example looks good to me.

@YuriSizov YuriSizov merged commit 53af94a into godotengine:master Aug 25, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@KoBeWi KoBeWi deleted the that's_not_how_you_do_set branch August 25, 2023 13:18
@YuriSizov YuriSizov added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.3.

@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Oct 24, 2023
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.

None yet

4 participants