-
-
Notifications
You must be signed in to change notification settings - Fork 192
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
Implemented enum #434
Implemented enum #434
Conversation
Thank you for putting effort in the improvement of the Yii framework. In order for the framework and your solution to remain stable in the future, we have a unit test requirement in place. Therefore we can only accept your pull request if it is covered by unit tests. Could you add these please? Thanks! P.S. If you have any questions about the creation of unit tests? Don't hesitate to ask for support. More information about unit tests This is an automated comment, triggered by adding the label |
Use GitHub actions
Unit test done |
2. To enum test added model->validation for rules testing
I've got few concerns:
|
In giiant enum use long time: https://github.com/schmunk42/yii2-giiant/blob/master/src/generators/model/Generator.php#L295 |
Please provide me MSSQL schema dump like https://github.com/yiisoft/yii2-gii/pull/434/files#diff-eef4be0410fa93517728588e0c838dd2f9afb86ff7148c7f7d7636de99df3b00R486 |
mssql don't have enum/set types and they not required in sql standards |
What do you mean by "strange"? These are legitimate concerns. You are just adding the feature but we will have to maintain it later on - I would like to at least be sure it works in most of the cases. And I'm not at the moment. I would like to see it tested with PostgreSQL since there is schema already in the tests added. And still no one said anything about the amount of additional code generated here (that is apart from you two guys, it's nice that you support each other like that). |
2. enum rule optimised 3. added for enums to model methods set[EnumField[To[EnumValue]
I woke up |
Apparently he went back to sleep. @samdark, what do you think about removing this PR from the 2.2.7 milestone and releasing the version? |
I define this initiative as illegal |
I prefer to believe that this was some kind of slang, as I don't understand English well. But if you were bothered by my joke, I apologize. My intention was just to clear the milestone so that a new version can be released. |
Don't take it seriously. When should I wake up? |
@uldisn time to wake up. Please add a line for CHANGELOG. Then I'll merge it. |
Done! |
Thank you! |
Generate: