Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TodoMVC spec fixes #10

Closed
wants to merge 3 commits into from
Closed

TodoMVC spec fixes #10

wants to merge 3 commits into from

Conversation

passy
Copy link

@passy passy commented Dec 14, 2014

Hey, I'm going through some of the test issues. I hope it's okay if I do this iteratively and not in one go.

This addresses how empty new todos are handled, renames is13 to isEnter (which is applicable from the JS code style guidelines we have) and removes some trailing spaces in the code.

@evancz
Copy link
Owner

evancz commented Dec 14, 2014

The trim branch is the one that is meant to pass the tests. Which one are you using?

@passy
Copy link
Author

passy commented Dec 14, 2014

@evancz D'oh, I was looking at master. Sorry!

@evancz
Copy link
Owner

evancz commented Dec 14, 2014

No worries, I gave it a silly name! I think the way the trailing spaces are removed is a bit buggy in this PR as it still gets placed in there with extra spaces, but it may be good to bring the approach over from the other branch. Not sure, I'm not really going for spec on the master branch, more about simplicity.

@passy
Copy link
Author

passy commented Dec 14, 2014

Yeah, I just saw your comment and didn't realize that it was on a different branch when I looked at it later. I'm still seeing a couple of test failures, but let me post that in a separate issue.

@passy passy closed this Dec 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants