Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Renamed users (admin) html files to follow the conventions of the Article module #823

Closed
wants to merge 0 commits into from

Conversation

vaucouleur
Copy link
Contributor

Renamed users (admin) files to follow the naming convention of the Article module:

  • user-edit.client.view.html -> edit-user.client.view.html
  • user-list.client.view.html -> list-users.client.view.html
  • user.client.view.html -> view-user.client.view.html

Discussion: #817

@codydaig
Copy link
Member

LGTM

@mleanos
Copy link
Member

mleanos commented Aug 19, 2015

@vaucouleur What about using the dot notation? for instance, edit.user.client.view.html

@trainerbill
Copy link
Contributor

@mleanos +1 That is what I would like to see and I think @lirantal agrees.

@vaucouleur
Copy link
Contributor Author

@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.

@trainerbill
Copy link
Contributor

@vaucouleur You have my vote on action.module.client.view.html

but I am not a contributor.

@codydaig
Copy link
Member

@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?

@lirantal
Copy link
Member

No objections, I'm all up for the dot notation.

@lirantal lirantal added this to the 0.4.x milestone Aug 19, 2015
@lirantal lirantal self-assigned this Aug 19, 2015
@vaucouleur
Copy link
Contributor Author

The commits are squashed, and both the user admin and the article modules now follows the convention:
verb.object.client.view.html

@mleanos
Copy link
Member

mleanos commented Aug 20, 2015

LGTM 👍 Tested and confirmed this is working.

Thanks @vaucouleur

@vaucouleur
Copy link
Contributor Author

@mleanos Thanks to you for double checking this !

@rhutchison
Copy link
Contributor

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

@codydaig codydaig mentioned this pull request Aug 20, 2015
@rhutchison rhutchison mentioned this pull request Aug 20, 2015
15 tasks
@mleanos
Copy link
Member

mleanos commented Aug 21, 2015

@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.

@lirantal
Copy link
Member

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.

@rhutchison
Copy link
Contributor

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.

@vaucouleur
Copy link
Contributor Author

@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.

  • change.profile-picture.client.controller.js

... versus ...

  • change.profile.picture.client.controller.js

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.

@lirantal
Copy link
Member

I don't have a strong argument to support either so I'm fairly native.
Though is there a common style for that which other frameworks/projects adopt? I'm sure we're not inventing the wheel here.

@rhutchison
Copy link
Contributor

I don't care either way, just want consistency.

https://github.com/johnpapa/angular-styleguide#style-y121

avenger-profile.directive.js

@lirantal
Copy link
Member

I'm ok with having this view-user.client.view.html and this to complement it change-profile-picture.client.controller.js

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?

@rhutchison
Copy link
Contributor

sounds good, @vaucouleur what do you say?

@vaucouleur
Copy link
Contributor Author

@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.

@lirantal
Copy link
Member

Great.

@mleanos
Copy link
Member

mleanos commented Aug 23, 2015

This is perfect. Thanks for getting this sorted out guys.

@vaucouleur vaucouleur closed this Aug 24, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants