-
-
Notifications
You must be signed in to change notification settings - Fork 546
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
secret-handshake: Add canonical data (revives #159) #489
Conversation
"expected": ["jump"] | ||
}, | ||
{ | ||
"description": "reversing no actions still gives the no actions", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the no?
I have to not use the term "reapply" since I have in the past used it to mean "revert a revert commit", so I don't want to overload the term. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description file mentions 9 as an example. Where possible I like to include a test for any examples included in the description file.
Also, the description file still mentions the binary-string conversion, which you have removed. So we should remove that from the description file.
Even though I've approved the current version, I also agree with @IanWhitney 's points. |
I've a decision to make. Situation:
I could do one of
I think it's best to make the README use 3 and 19. Coming right up. |
Implementing tracks: * https://github.com/exercism/xcsharp/blob/master/exercises/secret-handshake/SecretHandshakeTest.cs * https://github.com/exercism/xecmascript/blob/master/exercises/secret-handshake/secret-handshake.spec.js * https://github.com/exercism/xfsharp/blob/master/exercises/secret-handshake/SecretHandshakeTest.fs * https://github.com/exercism/xgo/blob/master/exercises/secret-handshake/secret_handshake_test.go * https://github.com/exercism/xhaskell/blob/master/exercises/secret-handshake/test/Tests.hs * https://github.com/exercism/xjavascript/blob/master/exercises/secret-handshake/secret-handshake.spec.js * https://github.com/exercism/xlua/blob/master/exercises/secret-handshake/secret-handshake_spec.lua * https://github.com/exercism/xobjective-c/blob/master/exercises/secret-handshake/SecretHandshakeTest.m * https://github.com/exercism/xperl5/blob/master/exercises/secret-handshake/handshake.t * https://github.com/exercism/xpython/blob/master/exercises/secret-handshake/handshake_test.py * https://github.com/exercism/xruby/blob/master/exercises/secret-handshake/secret_handshake_test.rb * https://github.com/exercism/xscala/blob/master/exercises/secret-handshake/src/test/scala/SecretHandshakeTest.scala * https://github.com/exercism/xswift/blob/master/exercises/secret-handshake/SecretHandshakeTest.swift Summary of tests among these languages: * All language tracks test: 1, 2, 4, 8, 3, 19, 31. * Go, Haskell, Objective C, Scala, and Swift additionally test 0. * Go is the only track that tests 32, 33. * Python is the only track that tests 9, 22, 5, -9. * Python is the only track that tests action -> number conversions. * Haskell, Python, and Scala test strings (string binary -> number conversion) * ECMAScript and JavaScript test strings (only that strings are rejected) * Perl and Ruby test strings (that they are accepted but result in an empty array) Closes #159 by way of replacement. Closes https://github.com/exercism/todo/issues/145 Changes since #159: * remove decode and composition cases Discussion in #159 indicates that the decode step should be removed. Since composition includes decode, that implies composition should be removed as well. * Test only integers Discussion in #159 indicates we don't do binary conversions, so all string inputs are geeting removed. * Test only positive integers It's recommended to use unsigned integers for bitwise operations. * describe importance of each test * All actions is 15 (1111), not 16 (10000) * Add cases for reverse singleton and nothing
We'd like to use 3 and 19 to match up with what's in the test cases already used by most languages. We also do not want to do binary to integer conversions, by discussion in #159. This covers most tasks in #335, though I would probably still leave #335 open because explicit discussion of numbers larger than 11111 has not been added.
The commits for the canonical-data.json have been squashed down to one, and the README change added as a separate commit. I think the JSON change and the README change should be kept as separate commits? I don't know, I could be convinced either way on that one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the separate commits.
Add the bookkeeping module to accumulate_test.rb Add bookkeeping to example.rb
The time has come to help the abandoned #159 get merged.
Implementing tracks:
Summary of tests among these languages:
Closes #159 by way of replacement.
Closes exercism/todo#145