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 Export Enum Example in c_sharp_exports.rst ( Line 458) #10832

Merged
merged 1 commit into from
Apr 6, 2025

Conversation

DeerUwU
Copy link
Contributor

@DeerUwU DeerUwU commented Apr 3, 2025

changed line 458:
public MyEnum MyEnum { get; set; }
to:
public MyEnum MyEnumCurrent { get; set; }

because it throws an error if you reuse the already declared name

changed line 458:
    public MyEnum MyEnum { get; set; }
to:
    public MyEnum MyEnumCurrent { get; set; }

because it throws an error if you reuse the already declared name
@AThousandShips AThousandShips changed the title Fixed Export Enum Example in c_sharp_exports.rst ( Line 458) Fix Export Enum Example in c_sharp_exports.rst ( Line 458) Apr 3, 2025
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

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

It does fix an error. "MyEnumCurrent" seems like a fine name, unless we have a more specific naming convention for cases like this.

(We could also avoid the naming problem entirely by using a more concrete example than "MyEnum" and "Thing1", but that seems like a larger change to the whole page we could make later.)

@tetrapod00 tetrapod00 requested a review from a team April 3, 2025 18:11
@tetrapod00 tetrapod00 added bug topic:dotnet area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.3 cherrypick:4.4 labels Apr 3, 2025
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.

Thanks. The change looks good.

It's usually idiomatic to use the same name as the type for the properties, but also because it's recommended to avoid nested types. In the case of this example, since the enum looks like it's defined as a nested type the name will conflict with the property name so it makes sense to use a different name. I don't have any strong opinion on what the names should be.

@skyace65 skyace65 merged commit 8343a18 into godotengine:master Apr 6, 2025
1 check passed
@skyace65
Copy link
Contributor

skyace65 commented Apr 6, 2025

Thanks! And congrats on your first merged PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:manual Issues and PRs related to the Manual/Tutorials section of the documentation bug cherrypick:4.3 cherrypick:4.4 topic:dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants