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

MyCLabs enum equals + keeping methods #4645

Merged
merged 4 commits into from
Sep 4, 2023

Conversation

K0nias
Copy link
Contributor

@K0nias K0nias commented Aug 4, 2023

Unfortunately I'm not sure of purpose of rules-tests/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector/Fixture/usage_of_constant.php.inc and rules-tests/Php81/Rector/MethodCall/MyCLabsMethodCallToEnumConstRector/Fixture/usage_of_constant.php.inc test cases.

Why it should be possible to refactor non existing constants to native enum when this enum will not have those cases?

Closes rectorphp/rector#8051

@samsonasik
Copy link
Member

failing test case need to be updated, FQCN result seems expected

@K0nias
Copy link
Contributor Author

K0nias commented Aug 4, 2023

I know that test cases need to be resolved. But I dont know how at this moment. In my opinion they are useless and can be deleted. But I am not author of thoses tests so maybe I dont see the proper use case for them.

@K0nias
Copy link
Contributor Author

K0nias commented Aug 4, 2023

I removed that failing tests cases because I think that they were usesless. There is no need to refactor non existing enum constatns to new native cases. If there is use case for them please let me know. I will restore them and properly fix code to pass tests

@K0nias
Copy link
Contributor Author

K0nias commented Aug 29, 2023

I cant figure out why this job https://github.com/rectorphp/rector-src/actions/runs/6013250808/job/16310321369?pr=4645 failing when my changes arent relevent to this check.

@samsonasik
Copy link
Member

@K0nias I restarted CI build

@TomasVotruba
Copy link
Member

CI looks good now 👍 Is this ready to merge or something missing?

@K0nias
Copy link
Contributor Author

K0nias commented Sep 4, 2023

From me it is ready to merge

@TomasVotruba
Copy link
Member

Thank you 👍

@TomasVotruba TomasVotruba merged commit 7571792 into rectorphp:main Sep 4, 2023
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants