Skip to content

isogram: update solution to pass current tests.#669

Merged
Insti merged 1 commit intoexercism:masterfrom
Insti:Fix_isogram_solution
Jun 8, 2017
Merged

isogram: update solution to pass current tests.#669
Insti merged 1 commit intoexercism:masterfrom
Insti:Fix_isogram_solution

Conversation

@Insti
Copy link
Copy Markdown
Contributor

@Insti Insti commented Jun 8, 2017

#664 updated the isogram tests but not the example solution.
For some reason Travis CI, which would have detected this, did not run on that PR.

This patch updates the example solution to pass the updated tests.


class Isogram
def self.is_isogram?(str)
def self.isogram?(str)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do use "string" as a name. Better use phrase or word or whatever the most general "language" thing is that is wanted, rather than a programming thing.

Yet, even without that change, the example is not shown actively, so it would be acceptable to the "the problem is solvable in this language" goal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The argument name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh, you were commenting on an aspect of the example solution that has not been changed as part of this PR.

This PR is just trying to make the existing example pass the current tests, if you want to improve the example solution this can be done in another PR.

@alexdebugs
Copy link
Copy Markdown
Contributor

Sorry I should have caught that as well, that's my fault.

@kotp
Copy link
Copy Markdown
Member

kotp commented Jun 8, 2017

@deohtee are you running the git hooks when you develop these?

@Insti Insti merged commit 4567380 into exercism:master Jun 8, 2017
@Insti Insti deleted the Fix_isogram_solution branch June 8, 2017 22:02
@alexdebugs
Copy link
Copy Markdown
Contributor

I am not. Is there any documentation on it? I tried looking for some just now but couldn't find any ☹️

@Insti
Copy link
Copy Markdown
Contributor Author

Insti commented Jun 9, 2017

@kotp?

The git hooks are not necessary to trigger Travis CI - only for local checks.

@Insti
Copy link
Copy Markdown
Contributor Author

Insti commented Jun 9, 2017

@deohtee the git hook documentation is here: https://github.com/exercism/xruby#pull-requests

@kotp
Copy link
Copy Markdown
Member

kotp commented Jun 10, 2017

Right... the hooks locally though can help contributors confirm the tests...

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