Removed globals, "Controller as" Syntax, Simplified functions

[dmitriz]

• Removed the global todomvc as it promotes a bad practice. Used modular approach instead. All modules are now visible in app.js and named as their files, following the principle of least surprise.
• Enclosed in anonymous functions as recommended in https://github.com/tastejs/todomvc/blob/master/codestyle.md
• Replaced the "$scope soup" by the recommended "Controller as" syntax. Improves readability of the template by making clear distinction between controller-defined properties/methods and the rest, among other benefits. All properties/methods are now named exactly the same in both template and controller.
• Simplified functions and watchers. No more need to use $filter in controller, eliminated unnecessarily complexity.
• More consistent naming. newTodo is made a todo, rather than a todo title.

[mention-bot]

By analyzing the blame information on this pull request, we identified @passy, @IgorMinar and @sindresorhus to be potential reviewers

[dmitriz]

Fixed the space warning.

Can change var TC = this; to var self = this; if needed, but this is sort of killing part of the improved readability.

[samccone]

Thanks for the PR @dmitriz I left come code review comments!

[samccone]

@dmitriz mind fixing the jscs errors (which is causing CI to fail)

As for the self complaint from jscs, as you pointed out what you are doing is idiomatic angular, so lets just disable that specific rule via an inline comment jscs-dev/node-jscs#486

👍 thanks for this awesome work and let me know if you have any other questions...

---

on a side note we have several angular examples that could use this work ^

https://github.com/tastejs/todomvc/tree/master/examples/angularjs
https://github.com/tastejs/todomvc/tree/master/examples/angularjs_require

Would be supppper amazing if you wanted to update these as well 😄

[dmitriz]

Thanks! You are most welcome :)
Will have a look at the other examples once this one is done.

The 2 questions remain:

• How to undo the past commit elegantly?
• Which rule to disable for the weird "self=this" warning??? No luck googling :(

[samccone]

1. You will want to do an interactive rebase

Checkout this awesome walkthrough
https://robots.thoughtbot.com/git-interactive-rebase-squash-amend-rewriting-history

here is the rule
http://jscs.info/rule/safeContextKeyword

we can disable with

//jscs:disable safeContextKeyword
...<code>..
//jscs:enable

[dmitriz]

2. Thanks - it works (though the rule looks different)!

3. Great article but didn't reveal the trick - found it here: http://christoph.ruegg.name/blog/git-howto-revert-a-commit-already-pushed-to-a-remote-reposit.html

[samccone]

oh oh I did not realize that was your question (pushing to a remote) :) glad you got it

[samccone]

@dmitriz in the future you should be able to force push the update over this branch without having to open a new PR... no worries though

[dmitriz]

Was trying to stay safe but thanks for the head up :)

[dmitriz]

Actually now I remember why I didn't do "force push"!

In one of my PRs to the Angular UI Bootstrap project I've added my own Gulp wrapper on top of their Grunt tasks but they wanted me to remove it. So I erased it from the git history and force pushed back.

The result was the loss of all in-code comments by the maintainers and my replies to them on Github!
The only change was in the Gulpfile, so none of those files were affected and the last thing I expect was them to disappear. Yet they were all gone!

All commits were still there and you would think the in-code comments would stay referenced from them but this does not seem how Github handles them, unfortunately. I found our discussion there useful and expected to be able to reference it in the future, so it was quite a bad surprise to see them all gone :(

Since then I stay away from force push.