Skip to content

book-store: Implement exercise #762

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 1, 2018
Merged

book-store: Implement exercise #762

merged 1 commit into from
Jan 1, 2018

Conversation

jgr
Copy link

@jgr jgr commented Oct 30, 2017

Why?

Ruby Exercism was missing this exercise.

What?

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds functionality)
  • Breaking change (fix or enhancement that would cause existing functionality to change)
  • Gardening (code and/or documentation cleanup)

Checklist:

  • I have read the CONTRIBUTING document.
  • My change relies on a pending issue/pull request
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@Insti Insti changed the title WIP Implement exercise book-store book-store: Implement exercise [WIP] Oct 31, 2017
@Insti
Copy link
Contributor

Insti commented Oct 31, 2017

Thanks @jgr let us know if you need any help.

Out of interest: Which documentation have you referred to for making this PR?

@jgr
Copy link
Author

jgr commented Oct 31, 2017

Howdy @Insti! I'm following this guide to adding an exercism exercise to another language. I regret that I've lost the context of how I ended up on that document! Is it out of date?

@Insti
Copy link
Contributor

Insti commented Oct 31, 2017

Thanks. That is the most up to date document.
I want to remove the section about creating an empty WIP pull request. so I was interested if that was the document you'd gotten the idea from.

@rpottsoh rpottsoh mentioned this pull request Nov 4, 2017
Merged
@Insti
Copy link
Contributor

Insti commented Nov 6, 2017

Have you made any progress on this @jgr ?

@jgr
Copy link
Author

jgr commented Nov 8, 2017

@Insti yeah, sorry I haven't pushed anything up! I'm just wrapping my head around the generator setup now, I'll let you know if I have any questions.

@Insti
Copy link
Contributor

Insti commented Nov 8, 2017

Even partial commits are helpful, so we can tell you you're on the right track..

We should probably write a guide to creating new exercises and generators.

@jgr jgr force-pushed the add-book-store branch 4 times, most recently from b44b4ba to dcf439e Compare December 11, 2017 20:38
@jgr jgr changed the title book-store: Implement exercise [WIP] book-store: Implement exercise Dec 11, 2017
@jgr
Copy link
Author

jgr commented Dec 11, 2017

@Insti I'd love a review of this PR now, I think I have all the bits required of a new exercise. Here's a list of the files in the PR and how they were created, I look forward to your feedback!

  • config.json I added a section to this file, per this document.
  • exercises/book-store/.meta/generator/book_store_case.rb I wrote this, following the directions here.
  • exercises/book-store/.meta/solutions/.version I added this manually, because I saw it referenced in the README a few places. It isn't clear to me how it is used, or if I was supposed to add it manually or expect it to be generated somehow.
  • exercises/book-store/.meta/solutions/book_store.rb This is my sample solution.
  • exercises/book-store/README.md I generated this using the Configlet tool. It wasn't clear to me if I was supposed to do this myself or not.
  • exercises/book-store/book_store_test.rb I generated this using ./bin/generate book-store.

@Insti
Copy link
Contributor

Insti commented Dec 13, 2017 •
edited by kotp
Loading

Great, thanks. I'm a bit busy at the moment, I'll check this out later in the week.

Insti
Insti reviewed Dec 13, 2017
View reviewed changes
exercises/book-store/book_store_test.rb Outdated
require 'minitest/autorun'
require_relative 'book_store'

# Common test data version: 1.0.1 087ad69
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a new version of the canonical-data.
You probably need to update your problem-specifications repository and re-gererate the tests.

@Insti
Copy link
Contributor

Insti commented Dec 13, 2017

I had a quick look, and this all looks good 👍
Just the test data that needs updating to the latest version.

@jgr
Copy link
Author

jgr commented Dec 18, 2017

Thanks @Insti! I went and and pushed an update with updated tests.

@kotp kotp merged commit a88c904 into exercism:master Jan 1, 2018
@kotp
Copy link
Member

kotp commented Jan 1, 2018

Thank you @jgr!

kytrinyx added a commit that referenced this pull request Oct 12, 2022
@kytrinyx
Give credit to author of book-store exercise
721c670 
jdr implemented book-store in #762.

This is the last exercise that was missing authors in the
.meta/config.json.

Closes #1067
@kytrinyx kytrinyx mentioned this pull request Oct 12, 2022
Merged
kotp pushed a commit that referenced this pull request Oct 12, 2022
@kytrinyx @kotp
Give credit to author of book-store exercise
b036e25 
jdr implemented book-store in #762.

This is the last exercise that was missing authors in the
.meta/config.json.

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

@kotp kotp kotp approved these changes

+1 more reviewer

@Insti Insti Insti left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jgr @Insti @kotp