Skip to content

prime-factors: use cases of x-common #481

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

Merged
merged 2 commits into from
Feb 1, 2017
Merged

prime-factors: use cases of x-common #481

merged 2 commits into from
Feb 1, 2017

Conversation

petertseng
Copy link
Member

exercism/problem-specifications#513


reviewers: your opinion will be appreciated on whether to incorporate the test descriptions into the file.

  1. Do nothing.
  2. Include as comments.
  3. Include as an element of the tuple, then use it on them so thye become test descriptions.

@petertseng
Copy link
Member Author

(My inclination is go for 3, but I didn't want to spend the time if nobody else cared)

@abo64
Copy link
Contributor

abo64 commented Jan 31, 2017

3 is more consistent with most other test suites, but it is indeed questionable whether it is worth the effort.

Maybe create a separate issue, but then for checking all test suites if they include the descriptions?

@petertseng
Copy link
Member Author

I don't necessarily think we need descriptions for all exercises - roman-numerals doesn't really need any https://github.com/exercism/xhaskell/blob/master/exercises/roman-numerals/test/Tests.hs - unless exercism/problem-specifications#451 comes up with something good.

@rbasso
Copy link
Contributor

rbasso commented Feb 1, 2017

Considering that one of the goals of Exercism seems to be keeping some degree of cross-language homogeneity, I would favor option 3, even in cases where the descriptions do not add too much to the tests.

, ("prime number", 2, [2] )
, ("square of a prime", 9, [3, 3] )
, ("cube of a prime", 8, [2, 2, 2] )
, ("product of primes and non-primes", 12, [2, 2, 3] )
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

someone who is looking closely will see that there is a shortcut being taken in the indentation: the comma overhangs the 3 two lines below.

Copy link
Contributor

@rbasso rbasso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems great!

@petertseng petertseng merged commit 9975086 into exercism:master Feb 1, 2017
@petertseng petertseng deleted the prime-factors branch February 1, 2017 18:22
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.

3 participants