-
-
Notifications
You must be signed in to change notification settings - Fork 195
hspec-lens-person: Rewrite test to use hspec with fail-fast. #381
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
Conversation
|
||
bornStreet :: Born -> String | ||
bornStreet = undefined | ||
|
||
setCurrentStreet :: String -> Person -> Person | ||
setCurrentStreet = undefined | ||
renameStreets :: (String -> String) -> Person -> Person |
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.
Oh, I see. You made the stub file follow alphabetical order.
The old argument for having them in this order is that a plain set is probably easier than a transform (sorry that I forget the lens specific terminology), and if students implement from top to bottom, they should probably implement the easier ones first.
However, at this point the tests should be guiding the implementation order since the tests will be run in order and the first one will fail.
I'm okay with this, let me know if I missed anything in my reasoning and/or there are other concerns
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 top level functions should be defined in a solution specific order or if they should be alphabetical.
Alphabetical order usually makes finding things easier, and I usually do the same in the exports list, to match. But I don't have a very strong opinion on this.
I left the data declarations in topological order. I'm inconsistent. 😁
Should I revert the functions to their old ordering?
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.
Fixed. Just reverted to the old order. 👍
|
||
-- Valid values of Gregorian are those for which 'Data.Time.Calendar.fromGregorianValid' | ||
-- returns Just. | ||
data Gregorian = Gregorian { |
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.
yeah I never understood this; I just deleted it in my submission
- Rewrite tests to use `hspec`. - Remove unneeded data type `Gregorian` and rewrite stub.
hspec
.Gregorian
and rewrite stub.Related to #211 and #302.