Removing some magic to make naming conventions more sane #218

mikegrassotti · 2013-10-06T01:30:20Z

Related: #184

Still a wip, this is where steph and i left off when pairing earlier.

dustinfarris · 2013-10-06T02:33:01Z

So does this mean no hyphens in filenames going forward?

stefanpenner · 2013-10-06T02:51:03Z

@dustinfarris yes, that is the idea. The reasoning is, less ambiguity === a good thing.

I would be interested in hearing others thoughts on this, as I know I tend to prefer to avoid ambiguity + unneeded complexity.

thomasboyt · 2013-10-06T03:03:14Z

really can't see a case where allowing both would lead to ambiguity; the only "complicated" part of this is within the resolver, not anything that gets customized

CodeOfficer · 2013-10-06T06:52:30Z

I have been bitten many times trying to figure out where ember wants under_score, lowerCamelCase or dash-er-ized strings ... I'd love to see it only work one way here. Normalizing isn't always a feature.

stefanpenner · 2013-10-06T14:32:56Z

@thomasboyt I would prefer we make this strict for now, we can always loosen the constraints later.

thomasboyt · 2013-10-06T16:28:44Z

Module names can be either "snake case" (app/controllers/some_thing.js) or "dasherized" (app/controllers/some-thing.js), depending on your personal preferences.

This seems user-friendly and not confusing to me. If we had a solid technical reason for changing this, I'd be okay with it, but I'm not seeing it.

Here's the situation after this change:

Objects: camelCase
Helpers: dasherized
Components: force dasherized, this is going to be very confusing IMO
View helpers: {{view "snake_case"}}, {{partial "snake_case"}}, etc.

If we do this, we need to make it clear that view helpers are using module names (i.e. file names) and that components have different file name rooms (or autoconvert snake case component names to dasherized)

stefanpenner · 2013-10-06T16:33:16Z

The current implementation utilizes a hack I added to almond to inspect the "registry" to make the choice. The only other option I see is wrapping require is a try/catch and attempting the other variation.

Additionally, I see little win for supporting both.

stefanpenner · 2013-10-06T16:34:22Z

vendor/resolver.js


    if (parsedName.fullName === 'router:main') {
      // for now, lets keep the router at app/router.js
      return require(prefix + '/router');
    }

-    if (requirejs._eak_seen[normalizedModuleName]) {
+    try {


would be nice if require supported has

MajorBreakfast · 2013-10-08T07:57:18Z

We should use either camelCase, snake_case or dashes. The javascript world generally aggrees on using dashes for filenames. And with ember this seems to be a good idea, too. this.route('some-stuff');, SomeStuffRoute in app/routes/some-stuff.js with the nice www.example.com/some-stuff url. I don't really care how the files are named, because they don't appear in the final product anyway, but the documentation should have a clear recommendation. A few months ago we had this ambiguitiy with the handlebars helper syntax. It's far better now.

stefanpenner · 2013-10-17T01:48:48Z

@MajorBreakfast we aim to only use dashes. 0 ambiguity is very important.

type-of-thing@namespace:something/name-of-thing

stefanpenner · 2013-10-17T01:49:46Z

@mikegrassotti have any time to continue working on this one?

ianpetzer · 2013-10-17T03:58:44Z

@stefanpenner The WIP commit that started this discussion seemed to converting dasherized-filenames to underscored_file_names ...

ianpetzer · 2013-10-17T03:59:51Z

vendor/resolver.js

@@ -74,31 +58,33 @@ define("resolver",

    // allow treat all dashed and all underscored as the same thing
    // supports components with dashes and other stuff with underscores.
-    var normalizedModuleName = chooseModuleName(requirejs._eak_seen, moduleName);
+    var normalizedModuleName = Ember.String.underscore(moduleName);


This line also seems to indicate that you are moving to underscored names instead of dasherized names

on disk underscored, in ember dasherized.

MajorBreakfast · 2013-10-18T07:54:27Z

On disk: Dasherized folder names, snake case javascript file names (this one is new to me, but is the case in ember source)
Module names: dasherized
Methods, classes, etc. in javascript/json files: camel case
CSS classes: dasherized

Is that it?

stefanpenner · 2013-10-18T12:56:24Z

on disk: snake_cased everything
moduleNames: snake_cased (to reflect the disk)
fullNames (the container keys, needs, store.find(..)): dasherized as for the most part "vague" things in the browser seem dasherized.
javascript syntax: camel
css classes: dasherized

ianpetzer · 2013-10-18T13:06:44Z

Great.. Thanks for the clarificaiton

On Fri, Oct 18, 2013 at 2:56 PM, Stefan Penner notifications@github.comwrote:

on disk: snake_cased everything
moduleNames: snake_cased (to reflect the disk)
fullNames (the container keys, needs, store.find(..)): dasherized
javascript syntax: camel
css classes: dasherized

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/218#issuecomment-26593248
.

MajorBreakfast · 2013-10-18T15:09:46Z

Snake case module names might be a bad idea, because node modules are always dasherized. I think npm even enforces that.

stefanpenner · 2013-10-18T21:13:56Z

@MajorBreakfast interesting... then maybe we must dasherize

mikegrassotti · 2013-10-18T21:21:26Z

Despite the commit message I am not against dashes. Just as long as we are consistent.

—
Sent from Mailbox for iPhone

On Fri, Oct 18, 2013 at 5:14 PM, Stefan Penner notifications@github.com
wrote:

@MajorBreakfast interesting... then maybe we must dasherize

Reply to this email directly or view it on GitHub:
#218 (comment)

MajorBreakfast · 2013-10-19T06:16:56Z

File names often end up in urls and most sites prefer dashes there, too. So, modules AND file/folder names dasherized?

stefanpenner · 2013-11-07T15:18:22Z

@MajorBreakfast's idea -> https://gist.github.com/MajorBreakfast/7317003

Pradeek · 2013-11-28T01:42:42Z

I agree with @MajorBreakfast here. Why do we want file names to be underscored when module names have to be dasherized? If we want one convention to rule them all, we should be switching completely to dashes instead. Also, components (both Ember and web component spec) enforce dashes, so dashes should be the convention going forward.

stefanpenner · 2013-11-28T04:43:43Z

I'm for 1 convention to rule them all, dasherized in all the places would be fine with me. (i believe)

stefanpenner · 2014-02-27T23:52:46Z

this work has been pushed to https://github.com/stefanpenner/ember-jj-abrams-resolver

wip: pairing w steph

abb33dc

stefanpenner reviewed Oct 6, 2013
View reviewed changes

ianpetzer reviewed Oct 17, 2013
View reviewed changes

stefanpenner referenced this pull request in DavyJonesLocker/ember-appkit-rails Oct 29, 2013

Everything working except for router resolver

dea8bdc

This was referenced Feb 11, 2014

Added module examples to the Naming Conventions page as well as a table ... #482

Closed

Dasherize testing files ember-cli/ember-cli#15

Closed

Dasherize testing files #496

Closed

stefanpenner closed this Feb 27, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Removing some magic to make naming conventions more sane #218

Removing some magic to make naming conventions more sane #218

mikegrassotti commented Oct 6, 2013

dustinfarris commented Oct 6, 2013

stefanpenner commented Oct 6, 2013

thomasboyt commented Oct 6, 2013

CodeOfficer commented Oct 6, 2013

stefanpenner commented Oct 6, 2013

thomasboyt commented Oct 6, 2013

stefanpenner commented Oct 6, 2013

stefanpenner Oct 6, 2013

MajorBreakfast commented Oct 8, 2013

stefanpenner commented Oct 17, 2013

stefanpenner commented Oct 17, 2013

ianpetzer commented Oct 17, 2013

ianpetzer Oct 17, 2013

stefanpenner Oct 17, 2013

MajorBreakfast commented Oct 18, 2013

stefanpenner commented Oct 18, 2013

ianpetzer commented Oct 18, 2013

MajorBreakfast commented Oct 18, 2013

stefanpenner commented Oct 18, 2013

mikegrassotti commented Oct 18, 2013

@MajorBreakfast interesting... then maybe we must dasherize

MajorBreakfast commented Oct 19, 2013

stefanpenner commented Nov 7, 2013

Pradeek commented Nov 28, 2013

stefanpenner commented Nov 28, 2013

stefanpenner commented Feb 27, 2014

Removing some magic to make naming conventions more sane #218

Removing some magic to make naming conventions more sane #218

Conversation

mikegrassotti commented Oct 6, 2013

dustinfarris commented Oct 6, 2013

stefanpenner commented Oct 6, 2013

thomasboyt commented Oct 6, 2013

CodeOfficer commented Oct 6, 2013

stefanpenner commented Oct 6, 2013

thomasboyt commented Oct 6, 2013

stefanpenner commented Oct 6, 2013

stefanpenner Oct 6, 2013

Choose a reason for hiding this comment

MajorBreakfast commented Oct 8, 2013

stefanpenner commented Oct 17, 2013

stefanpenner commented Oct 17, 2013

ianpetzer commented Oct 17, 2013

ianpetzer Oct 17, 2013

Choose a reason for hiding this comment

stefanpenner Oct 17, 2013

Choose a reason for hiding this comment

MajorBreakfast commented Oct 18, 2013

stefanpenner commented Oct 18, 2013

ianpetzer commented Oct 18, 2013

MajorBreakfast commented Oct 18, 2013

stefanpenner commented Oct 18, 2013

mikegrassotti commented Oct 18, 2013

@MajorBreakfast interesting... then maybe we must dasherize

MajorBreakfast commented Oct 19, 2013

stefanpenner commented Nov 7, 2013

Pradeek commented Nov 28, 2013

stefanpenner commented Nov 28, 2013

stefanpenner commented Feb 27, 2014