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

isbn-verifier: add new exercise #910

Merged
merged 3 commits into from Oct 25, 2017

Conversation

Projects
None yet
2 participants
@creaaa
Contributor

creaaa commented Oct 24, 2017

Dear community,

Hi, I'm Masa, opened #906.
Sorry for delaying implementation and any inconvenience I've been causing.
I didn't mean that I closed #906. I didn't know PR gets closed when I delete rerated branch.
And now I send the files again.
I really appreciate taking your time. Thanks.

Masa

@tleen

Almost there! We appreciate your patience working through this exercise. A few small things.

* Generate a valid ISBN-13 from the input ISBN-10 (and maybe verify it again with a derived verifier)
* Generate valid ISBN, maybe even from a given starting ISBN## Source

This comment has been minimized.

@tleen

tleen Oct 25, 2017

Member

"## Source" should be a new line. Did you make this file by hand or use a tool?

This comment has been minimized.

@creaaa

creaaa Oct 25, 2017

Contributor

I made this by configlet generate. And I included generated README.md in this PR.
There may be some incorrect process in my way...

This comment has been minimized.

@tleen

tleen Oct 25, 2017

Member

That is the right way to do it, there may be a bug somewhere with the source header.

func IsValidISBN(isbn string) bool {
// step 1

This comment has been minimized.

@tleen

tleen Oct 25, 2017

Member

These comments don't tell me much. It is self-evident that these are steps. Better would be why this step at this point rather than a label...

for _, test := range testCases {
observed := IsValidISBN(test.isbn)
if observed != test.expected {
t.Fatalf("IsValidISBN(%s) = %t, want %t (%s)",

This comment has been minimized.

@tleen

tleen Oct 25, 2017

Member

No need to repeat the function name here, a FAIL will let us know it is in "TestIsValidISBN" which makes the failing function simple to deduce. The output string may be better as:

t.Fatalf("got %t, want %t, %s (%s)",
  observed, test.expected, test.description, test.isbn)
@tleen

tleen approved these changes Oct 25, 2017 edited

Nice work! Thanks! one small thing...

@tleen

tleen requested changes Oct 25, 2017 edited

Can you try regenerating that README again? I did it locally and it turned out ok. I'll be in gitter for a little while if you want to do it together.

@creaaa

This comment has been minimized.

Contributor

creaaa commented Oct 25, 2017

Yes, I'm going to try, and report the result.

@creaaa

This comment has been minimized.

Contributor

creaaa commented Oct 25, 2017

Let me explain how to generate README.md. There must be some wrong way in it.

My file structure is below:

  • PROJECT_ROOT/
    • exercises/
      • problem-specifications/
      • isbn-verifier/
        • config/
          • exercise_readme.go.tmpl
        • exercises/
          • isbn-verifier/
    • configlet

Then I commanded ./configlet generate ./exercises/isbn-verifier/ --only isbn-verifier at project root.
README.md is generated in PROJECT_ROOT/exercises/isbn-verifier.

Could you tell me what is wrong with me?

@tleen

This comment has been minimized.

Member

tleen commented Oct 25, 2017

How did you get configlet?

@creaaa

This comment has been minimized.

Contributor

creaaa commented Oct 25, 2017

I get a binary from release page.
But I noticed I didn't use the latest version. (latest is ver3.6.0 but I was using ver 3.5.1).
This must cause problem... Sorry, I'll try again immediately.

@tleen

This comment has been minimized.

Member

tleen commented Oct 25, 2017

A few things look odd in your setup:

problem-specifications should be a sibling to the go track, not under it. Like this:

.
├── go
└── problem-specifications

and configlet should be obtained in the go track directory via the command bin/fetch-configlet, after that your go/bin directory should look like this:

bin/
├── configlet
├── fetch-configlet
└── test-without-stubs

Then from the root of your go track directory you call:

bin/configlet generate . --only isbn-verifier

This should result in the correct README being generated.

@creaaa

This comment has been minimized.

Contributor

creaaa commented Oct 25, 2017

Thanks for the kind suggestion. Let me try.

@creaaa

This comment has been minimized.

Contributor

creaaa commented Oct 25, 2017

Thank you so much!

@tleen

This comment has been minimized.

Member

tleen commented Oct 25, 2017

Yes! That's it. 😃 Thanks for sticking with it!

@tleen

tleen approved these changes Oct 25, 2017

@creaaa

This comment has been minimized.

Contributor

creaaa commented Oct 25, 2017

Thanks for the mentoring. I really appreciate your help for a long time.😌

@tleen tleen merged commit 48d1ee9 into exercism:nextercism Oct 25, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tleen

This comment has been minimized.

Member

tleen commented Oct 25, 2017

It's official 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment