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

Enum classes #39510

Merged
merged 9 commits into from
Apr 15, 2020
Merged

Conversation

ZhilkinSerg
Copy link
Contributor

@ZhilkinSerg ZhilkinSerg commented Apr 12, 2020

Summary

SUMMARY: None

Purpose of change

Add class to some enums (rename enum class members in the process, to remove prefixes)

Describe alternatives you've considered

Leave enums alone in a classless society.

Testing

Let it compile.

@ZhilkinSerg ZhilkinSerg added [C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style labels Apr 12, 2020
@jbytheway
Copy link
Contributor

It would be good to remove the (now-unnecessary) common prefix from the enumerator names.

If you don't want to do that by hand, then I had a plan to implement a clang-tidy check to do it at some point.

@anothersimulacrum
Copy link
Member

I introduce a new enumeration in #39387 should I convert it to an enum class as well?

@kevingranade
Copy link
Member

kevingranade commented Apr 13, 2020 via email

@ZhilkinSerg
Copy link
Contributor Author

I introduce a new enumeration in #39387 should I convert it to an enum class as well?

Yes, it should not be any harder to use than plain enums in most cases.

It would be good to remove the (now-unnecessary) common prefix from the enumerator names.

If you don't want to do that by hand, then I had a plan to implement a clang-tidy check to do it at some point.

I was thinking about renaming them too. Will do.

@jbytheway
Copy link
Contributor

Yes, I want to discuss with everyone, but I think this would be a good thing to make mandatory.

Agreed, I think it should be the default to do so, unless some unusual situation dictates otherwise. Certainly for enums at global scope.

@kevingranade kevingranade merged commit 14415de into CleverRaven:master Apr 15, 2020
fengjixuchui added a commit to fengjixuchui/Cataclysm-DDA that referenced this pull request Apr 15, 2020
Merge pull request CleverRaven#39510 from ZhilkinSerg/enum-classes-20…
@ZhilkinSerg ZhilkinSerg deleted the enum-classes-2020-04-11 branch April 15, 2020 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` Code: Infrastructure / Style / Static Analysis Code internal infrastructure and style
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants