-
-
Notifications
You must be signed in to change notification settings - Fork 217
Update cmdmenu.sma #241
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
Update cmdmenu.sma #241
Conversation
luxxxoor
commented
May 5, 2015
- Changed format with formatex where needed.
- Removed many hardcoded contents in plugin.
- Added semicolons.
plugins/cmdmenu.sma
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you "expand" this condition? It was fine before. No reason to change it.
|
Why did you close the PR? |
|
I pressed the wrong button from phone, i think the "expand" was needed to understand the code more easier |
|
I think honestly it does the reverse. You see all conditions on the same level, which is more readable, and unless it's really needed (like logic issue), I would not change them. |
|
For me it is a lot easier to understand the code if it is explained, but if you think the expand will do the reverse i will return to the standard. And i want to know if all the expands are bad (not easy readable) or just some of them ? |
|
In this context, there is no meaning to do that. I would prefer you revert it. Also this is not of matter of bad or something, when conditions are simple and can be aligned to have a better overview, this is more readable. If conditions are complex or long, you can expand so it's easier to understand ; just a matter of context. Squashing your commits to one would be appreciated. |
|
Why did you revert ALL the changes about the added parenthesis? Did I mention you to revert that? I've SPECIFICALLY pointed above the lines where EXACTLY I would expect some changes... If I did not commented on the others lines, it means It was okay, why are you doing unnecessary changes now? |
|
Now is ok ? If it's ok i will squash the commits. |
|
Yes. |
Changed format with formatex where needed, removed many hardcoded contents in plugin and added semicolons.
|
Thanks! |