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

Use enum abstract where available. #122

Closed
wants to merge 2 commits into from

Conversation

player-03
Copy link
Contributor

The old syntax is deprecated as of Haxe 4.3.

This syntax was added in Haxe 4.0, and Haxe 4.3 has begun emitting warnings.
Comment on lines 12 to 13
#if !haxe4 @:enum #end
private #if haxe4 enum #end abstract IsAsync(Int) {
Copy link
Contributor

@AlexHaxe AlexHaxe Aug 15, 2023

Choose a reason for hiding this comment

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

I find using two surgical conditional sections while solving the issue it also make it way harder to read.

surely noone is a fan of duplicating code, but I think in this case it would be much clearer and more acceptable if it was something like:

#if haxe4
private enum abstract IsAsync(Int) {
	var Yes = 1;
	var No = 0;
	var Unknown = -1;
}
#else
@:enum
private abstract IsAsync(Int) {
	var Yes = 1;
	var No = 0;
	var Unknown = -1;
}
#end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this?

Copy link
Contributor

Choose a reason for hiding this comment

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

personally I don't like conditionals mixed with code lines, but I can't speak for the maintainers. I don't know what they find acceptable / prefer.

@player-03
Copy link
Contributor Author

Fixed in #124.

@player-03 player-03 closed this Dec 6, 2023
@player-03 player-03 deleted the enum-abstract branch December 6, 2023 22:06
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.

2 participants