-
Notifications
You must be signed in to change notification settings - Fork 2k
Renamed users (admin) html files to follow the conventions of the Article module #823
Conversation
LGTM |
@vaucouleur What about using the dot notation? for instance, |
@mleanos @trainerbill Dots or dashes, any choice is fine with me. I just pointed out that it seemed wise to be consistent across modules. The Article module uses dashes, and there is a lot of "generated modules" out there that use the Article convention. If we do move to dots, we should also rename the Article html templates. Is that what we want ? If so, I can amend the PR. |
@vaucouleur You have my vote on action.module.client.view.html but I am not a contributor. |
@vaucouleur Sounds like the consensus is action.module.client.view.html (edit.user.client.view.html). If you could update and squash your commits, it should be good to go. Any objections @lirantal? |
No objections, I'm all up for the dot notation. |
The commits are squashed, and both the user admin and the article modules now follows the convention: |
LGTM 👍 Tested and confirmed this is working. Thanks @vaucouleur |
@mleanos Thanks to you for double checking this ! |
Not to throw a wrench in to this, but I preferred the naming convention of users admin to following the naming convention of the state. Also, you have other files in the project that should be considered if you want to set a convention. Example: https://github.com/meanjs/mean/tree/master/modules/users/client/controllers/settings |
@rhutchison I see your point, but I think for views it makes more sense to have the verb first in the naming convention. The reason it makes sense to have the states named with their module/feature first, is the hierarchical nature of states. Views don't follow that type of pattern, IMO. |
right, I'm still ok with the naming convention proposed here unless @rhutchison want to suggest some higher level more consistent naming convention across the project. |
I am fine with flipping the words, but my point about inconsistency remains. This PR was updated to implement dot notation, where-as the rest of the project is still using dash. If you are going to switch to dot, then it should be across the project. |
@rhutchison This is a good point, consistency was my main initial concern. Ideally the file naming convention should be consistent across the project. I would be happy to amend the PR and apply the same naming convention to all html pages, if there is a consensus about this. Something else: some pages have 2 words for the object name, for example "profile picture". Those words can be either separated by a dash (to denote "a close binding between those words"), or to also use a dot.
... versus ...
My preference would be to keep a dash in this case between "profile" and "picture". But again this is a minor point, any of those choices is probably good as long as it is used consistently. |
I don't have a strong argument to support either so I'm fairly native. |
I don't care either way, just want consistency. https://github.com/johnpapa/angular-styleguide#style-y121
|
I'm ok with having this So this stays consistent where the first part of the filename (usually the verb/action) is formatted with a dash, and then the following filename info is with dots. Sounds good? |
sounds good, @vaucouleur what do you say? |
@lirantal @rhutchison That's just fine. I will fix the PR with this pattern, and I will extend the PR to all html pages. Thanks for your feedback. |
Great. |
This is perfect. Thanks for getting this sorted out guys. |
Renamed users (admin) files to follow the naming convention of the Article module:
Discussion: #817