-
-
Notifications
You must be signed in to change notification settings - Fork 195
Add Pangram Exercise #511
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
Add Pangram Exercise #511
Conversation
085c0dc
to
3cdeaea
Compare
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.
Everything seems reasonable, since the implementation is pretty straightforward, thanks for that! I think I would ask the other maintainers of a few administrative matters (noted as line comments)
exercises/pangram/stack.yaml
Outdated
@@ -0,0 +1 @@ | |||
resolver: lts-8.6 |
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.
good time to ask now about whether there is a policy:
- Do we want to keep the same version across the same track?
- Or is it acceptable to let each exercise have its own version?
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.
Ah, I just read the instructions again and realised I may have misunderstood this bit:
stack.yaml
has just one line specifying the current Stack snapshot. We use the same resolver for all the exercises.
The first part suggests we should include the current Snapshot from the link given, but the second part says we should use the same resolver for all exercises!!!11one
I'd prefer consistency within the track, so either downgrade this one to lts-8.2
, or upgrade the others.
Perhaps we could fix the resolver version in a config file within xhaskell
and then create a bin in xhaskell/bin
that will generate the stack.yaml
for a given path? Starting with stack.yaml
, this could evolve to a file that would generate the full exercise structure.
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 current Stack snapshot
Yes, I guess that is ambiguous. Perhaps "the track's current Snack snapshot" would be more accurate.
Starting with
stack.yaml
, this could evolve to a file that would generate the full exercise structure.
Nice idea. We could start simple with a _template directory like https://github.com/exercism/xjava/tree/master/exercises/_template or https://github.com/exercism/xkotlin/tree/master/exercises/_template
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.
either downgrade this one to lts-8.2, or upgrade the others
Let's save the upgrade for its own PR that affects all exercises like #500. I think this PR for Pangram will be ready to merge with 8.2.
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.
done 👍
exercises/pangram/test/Tests.hs
Outdated
|
||
-- Test cases adapted from | ||
-- `exercism/x-common/exercises/pangram/canonical-data.json` | ||
-- on 2017-03-28. |
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 think at this point since there is a version at https://github.com/exercism/x-common/blob/master/exercises/pangram/canonical-data.json#L6 we should include it somehow, just not sure what format we want. "x-common version: 1.0.0" perhaps?
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'm not sure if the structure I used here was explicitly made standard/convention or simply accepted by adapting from other tests (I simply followed/copied from other tests).
That said, now that we have more data to include, I'm going to propose the following:
Adapted from
Source: exercism/x-common/exercises/pangram/canonical-data.json
Version: 1.0.0
Date: 2017-03-28
Test cases
seems redundant since that's what the file is about.
I think this exercise is fairly trivial to be implemented. Plus, it is good to have more exercises between Bob and Run Length Encoding, since the first is much simpler than the latter.