Skip to content

Conversation

@Bashamega
Copy link
Contributor

related #2053

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2025

Thanks for the PR!

This section of the codebase is owned by @saschanaz - if they write a comment saying "LGTM" then it will be merged.

Copy link
Contributor

@saschanaz saschanaz left a comment

Choose a reason for hiding this comment

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

Nothing to do with mixin? LGTM with title change

@@ -0,0 +1 @@
enum ClientType name="ClientTypes"
Copy link
Contributor

Choose a reason for hiding this comment

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

The file name can be type-rename.kdl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, but i have named it typerename.kdl, to follow the same format as the other files

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm? typerename is not single word while other file names are single words

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will update it

@Bashamega Bashamega changed the title Feature: Rename mixins in KDL Feature: Rename enum in KDL Aug 2, 2025
@Bashamega
Copy link
Contributor Author

Nothing to do with mixin? LGTM with title change

I have changed it

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2025

Sorry @Bashamega, you don't have access to these files:

@saschanaz
Copy link
Contributor

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2025

There was an issue merging, maybe try again saschanaz. Details

@@ -0,0 +1 @@
enum ClientType name="ClientTypes"
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW you can also skip quote.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you like me to remove them? I find it more readable with quotes, especially for properties

Copy link
Contributor

Choose a reason for hiding this comment

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

enum ClientType name="ClientTypes"
enum ClientType name=ClientTypes

The quote gives a strong signal of "I'm string", but technically everything is string here, including enum (you can do "enum" and get the same result). So I prefer skipping it for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@saschanaz
Copy link
Contributor

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2025

There was an issue merging, maybe try again saschanaz. Details

@saschanaz
Copy link
Contributor

LGTM

@github-actions github-actions bot merged commit 3082e4f into microsoft:main Aug 2, 2025
6 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2025

Merging because @saschanaz is a code-owner of all the changes - thanks!

@Bashamega Bashamega deleted the rename-enums branch August 4, 2025 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants