Skip to content

Update isogram tests to v1.6#749

Merged
petertseng merged 1 commit intoexercism:masterfrom
tejasbubane:update-isogram-tests
Oct 30, 2018
Merged

Update isogram tests to v1.6#749
petertseng merged 1 commit intoexercism:masterfrom
tejasbubane:update-isogram-tests

Conversation

@tejasbubane
Copy link
Member

No description provided.

Copy link
Member

@petertseng petertseng left a comment

Choose a reason for hiding this comment

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

the test addition is, of course, in order and good.

I have one change to request with regard to the version number.

In addition, I have an administrative request, which is that changes that follow problem-specifications should specify the relevant original PRs, which in this case are exercism/problem-specifications#1308 and exercism/problem-specifications#1307 so that it is possible to understand why the test cases are added. I will be editing that into the commit message when it is merged, so you don't need to do this for this PR, but you will save time for any future PRs you chose to make should you be so kind as to provide this.

@@ -1,5 +1,5 @@
name: isogram
version: 1.4.0.4
version: 1.6.0.1
Copy link
Member

Choose a reason for hiding this comment

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

By the https://github.com/exercism/haskell/blob/master/README.md#exercise-versioning , the last component always increases independently of the other numbers, so this has to be 1.6.0.5

While I understand that starting over from 1.6.0.1 would usually not be in conflict with the goals of the last version component (distinguish between multiple versions that are based on the same problem-specifications version), I guess it's important if we ever decided to go to, say 1.7.0.1 and then decided to revert back to 1.6.0.x - then in that case we have to look back in history to see what x should be. Saves the work of doing so if we instead just always increase it.

@tejasbubane
Copy link
Member Author

@petertseng Thanks! I will keep these in mind going forward. I have made both the changes - version number and commit message.

@petertseng petertseng merged commit df267b6 into exercism:master Oct 30, 2018
@petertseng
Copy link
Member

Fantastic, thank you!

@tejasbubane tejasbubane deleted the update-isogram-tests branch October 30, 2018 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants