Skip to content

Adds generator for queen attack exercise#487

Merged
kotp merged 1 commit intoexercism:masterfrom
samjonester:generators
Nov 21, 2016
Merged

Adds generator for queen attack exercise#487
kotp merged 1 commit intoexercism:masterfrom
samjonester:generators

Conversation

@samjonester
Copy link
Copy Markdown
Contributor

@samjonester samjonester commented Nov 8, 2016 •
edited
Loading

The generator for this test uses the interface with methods test_name, skipped, workload. It originally comes from issue #439

@Insti
Copy link
Copy Markdown
Contributor

Insti commented Nov 8, 2016 •
edited
Loading

Thanks @samjonester

  1. It's useful if you add a bit more information about the PR to the commit message and the first post of the pull request.

    Things like: What you've added, how it differers from what was there before, if there were any special challenges/differences from the 'average' test generator.

  2. The build seems to be failing due to Rubocop errors: Is there a problem with the file or is Rubocop wrong in this case?

3.57s$ rubocop -fs -D
== lib/queen_attack_cases.rb ==
E: 51: 38: unexpected token tCOLON
(Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops)
E: 51: 43: unexpected token tRPAREN
(Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops)

@samjonester
Copy link
Copy Markdown
Contributor Author

samjonester commented Nov 8, 2016

@Insti Thanks for the feedback! I've fixed the rubocop issue, and provided some more documentation. Let me know if anything else isn't up to snuff :)

@Insti Insti mentioned this pull request Nov 9, 2016
Closed
@Insti
Copy link
Copy Markdown
Contributor

Insti commented Nov 9, 2016

Great, thanks @samjonester, someone will review it properly soon, but it looks good at first-glance.

bmulvihill
bmulvihill reviewed Nov 10, 2016
View reviewed changes
Comment thread exercises/queen-attack/queen_attack_test.rb
assert_equal [7, 3], queens.black
# Test data version:
# 82eb00d
class QueenTest < Minitest::Test
Copy link
Copy Markdown
Contributor

@bmulvihill bmulvihill Nov 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like there is no space between tests in this file which could make it hard for the end user to follow.

Copy link
Copy Markdown
Contributor Author

@samjonester samjonester Nov 14, 2016

Choose a reason for hiding this comment

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

Taken care of :)

@samjonester
Adds generator for queen attack exercise
740efe3 
Queen attack had tests being built by hand. This creates a generator for
the exercise.

Tests related to the rendering of the board have been removed. Tests now
come solely from x-common.
bmulvihill
bmulvihill approved these changes Nov 14, 2016
View reviewed changes
Copy link
Copy Markdown
Contributor

@bmulvihill bmulvihill left a comment

Choose a reason for hiding this comment

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

This looks good to me

@kotp kotp merged commit d05dc36 into exercism:master Nov 21, 2016
@kotp
Copy link
Copy Markdown
Member

kotp commented Nov 21, 2016

Thanks @samjonester

@Insti Insti removed the in progress label Nov 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@bmulvihill bmulvihill bmulvihill approved these changes

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.

4 participants

@samjonester @Insti @kotp @bmulvihill