Skip to content
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

Build the reps in the show commands so they can be described #970

Merged
merged 2 commits into from Oct 29, 2016

Conversation

@cdchapman
Copy link
Contributor

@cdchapman cdchapman commented Oct 28, 2016

Summary

The show-{data,rules} commands do not load or describe the item reps. This pull request builds the reps in the setup of the command so they can be described.

To do

  • Tests for show-rules
  • Tests for show-data

Detailed description

The show-{data,rules} commands do not load or describe the item reps, so the methods that use the reps in these commands are actually operating on an empty Nanoc::Int::ItemRepRepo and do not output anything helpful.

I'm not sure whether this is the best way to accomplish loading the reps, but this simple change enables these two commands to work as expected.

About the lack of tests, the existing test for show-rules creates stubs for the item reps, and does not test the actual setup of the command. I'm willing to create tests for this if you can give me some direction. Looking closer at the tests (sorry I goofed), the setup is mocked. I've added a block to expect that build_reps is called.

Anyways, thanks for a great project!

@cdchapman cdchapman changed the title Build the reps so they can be described Build the reps in the show commands so they can be described Oct 28, 2016
@ddfreyne ddfreyne added this to the 4.3.7 milestone Oct 29, 2016
@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Oct 29, 2016

Thanks for the fix!

The issue with item reps not being filled in has popped up before, and I’ll see what I can do to make this less likely to happen.

Loading

@ddfreyne ddfreyne merged commit c276a4c into nanoc:master Oct 29, 2016
1 check passed
Loading
ddfreyne added a commit that referenced this issue Oct 29, 2016
@ddfreyne
Copy link
Member

@ddfreyne ddfreyne commented Oct 29, 2016

I added two additional high-level regression tests in 7ffd07c.

Loading

@cdchapman
Copy link
Contributor Author

@cdchapman cdchapman commented Oct 29, 2016

Thank you!

Loading

@cdchapman cdchapman deleted the bug/load-reps-in-show-commands branch Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants