Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upComprehensive rewrite of Char tests #158
Conversation
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jonathanhefner
Feb 3, 2015
Contributor
I'm... not sure what happened to the build. Travis CI says the build timed out, but the tests run swiftly (and pass) locally. Is it possibly a glitch in the build system? Is there a way to trigger another attempt?
|
I'm... not sure what happened to the build. Travis CI says the build timed out, but the tests run swiftly (and pass) locally. Is it possibly a glitch in the build system? Is there a way to trigger another attempt? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jonathanhefner
Feb 3, 2015
Contributor
Looks like it was just a glitch.
In case anyone is curious, I was able to force another build by doing an empty git commit --amend and then a git push -f to my branch.
|
Looks like it was just a glitch. In case anyone is curious, I was able to force another build by doing an empty |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Why do you put single tokens in parentheses? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
jonathanhefner
Feb 3, 2015
Contributor
Just for consistency. I guess it also feels a little easier to eyeball with the parens acting as delimiters, but that's subjective.
If it's bad practice (or just ugly), I can strip them out.
|
Just for consistency. I guess it also feels a little easier to eyeball with the parens acting as delimiters, but that's subjective. If it's bad practice (or just ugly), I can strip them out. |
pushed a commit
that referenced
this pull request
Feb 4, 2015
evancz
merged commit 4825a2a
into
elm:master
Feb 4, 2015
1 check passed
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
evancz
Feb 4, 2015
Member
I'd generally consider it bad style to have extra parens, but I only care to the extent that people will use these tests as a guide for writing their own. I'm not sure what the right thing is here, but it feels weird.
In any case, things look good, thanks!
|
I'd generally consider it bad style to have extra parens, but I only care to the extent that people will use these tests as a guide for writing their own. I'm not sure what the right thing is here, but it feels weird. In any case, things look good, thanks! |
jonathanhefner commentedFeb 3, 2015
Like #150, I rewrote the Char tests to be more comprehensive. All exposed API functions are covered and tested with all alpha-decimal characters. The tests are also more structured, so it is easier to add tests for future corner cases (e.g.
toLocaleLowerin a Turkish locale).