Feature: camelCase - Githubissues

tetsuo commented 9 years ago

This is #85 merged with #86

Long story short: Lowercase the reference (head), and keep the rest (tail) untouched.

require('component/DoTheHarlemShake')

micky2be commented 9 years ago

Please guys, merge this asap. This is killing us.

tetsuo commented 9 years ago

@micky2be :d i was waiting for some authorized personnel to take a look at it :)

@timaschew @netpoetica what do u guys think? i haven't tested this extensively since i have only one project with mixed case dependency names, it has worked for me well on that one

19h commented 9 years ago

LGTM. @timaschew check?

I'll land locally and see what Jenkins says.

netpoetica commented 9 years ago

@tetsuo @timaschew For me, this doesn't effect me badly. This is the kind of thing I would have argued for a year ago but maintainers would have said no / it's bikeshedding / extra code to maintain, etc. 90% of the changes are just to add tests, the rest is just removal of forcing lookups to lowercase.

I can't think of anything this would break, and if tests pass everything should be good - but I am curious if there are other areas of the component that may not be compatible with this change.

I'm thinking something like:

remotes vs. locals - looks like tests are all for local components, no? Would this cause problems if user specifies a camelCased dependency whose name isn't camelCased in it's remote?
component.json "name" property, if it has variety of cases, will builder generate it properly? Will builder take "myComponent" and make it all lowercase, and then it wouldn't be requireable?

Those are the only things I can imagine being impacted, maybe you've already considered this?

One other thing I think that might be helpful is to add a list of acceptable example component names here and here - what do you guys think about that?

19h commented 9 years ago

Everything is working fine according to my testing, couldn't locate any regressions. Unless anyone can report a breaking change, I'll merge this for now.

P.s.: please squash your commits in future PRs, they're easier to locate and group visually if commits happen to have been made between your commits.

@timaschew prepush reported 2 fails (of 1599), but they don't correlate with this change. @netpoetica If you happen to have reservations against this, please feel free to revert.

micky2be commented 9 years ago

Would be nice to make a release now

19h commented 9 years ago

1.2.0 is tagged. @timaschew package and deliver! :boom:

timaschew commented 9 years ago

published on npm

micky2be commented 9 years ago

:+1:

componentjs / builder2.js

Feature: camelCase #87