Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Dota API: Str Heroes, heroes quotes, Items, Teams, Players #1174

Merged
merged 5 commits into from
May 19, 2018

Conversation

felipesousafs
Copy link
Contributor

As described in the commit message, added some heroes (and hero quotes), items, teams and players.

Copy link
Contributor

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

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

Just need to restore the rake task, otherwise 👍

tasks/test.rake Outdated
@@ -3,7 +3,7 @@ require 'rake/testtask'
Rake::TestTask.new do |t|
t.libs << "test"
t.libs << "."
t.test_files = FileList['test/test*.rb']
t.test_files = FileList['test/test_faker_dota.rb']
Copy link
Contributor

Choose a reason for hiding this comment

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

This eliminates all the rest of the tests and should be put back to what's on master.


def test_hero
assert_match(/\w+/, @tester.hero)
puts @tester.hero
Copy link
Member

@vbrazo vbrazo May 13, 2018

Choose a reason for hiding this comment

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

Please remove the instance method puts to keep the tests clean


def test_item
assert_match(/\w+/, @tester.item)
puts @tester.item
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Please do not forget to remove the other puts below.

@vbrazo
Copy link
Member

vbrazo commented May 13, 2018

Travis is red because of this:

Error: test_determinism(TestDeterminism): RuntimeError: Faker::Dota.quote raised "wrong number of arguments (given 0, expected 1)"
/faker/test/test_determinism.rb:36:in `rescue in store_result'
/faker/test/test_determinism.rb:33:in `store_result'
/faker/test/test_determinism.rb:14:in `block in test_determinism'
     11:   def test_determinism
     12:     Faker::Config.random = Random.new(42)
     13:     @all_methods.each_index do |index|
  => 14:       store_result @all_methods[index]
     15:     end
     16:     @first_run.freeze
     17:     Faker::Config.random = Random.new(42)
/faker/test/test_determinism.rb:13:in `each_index'
/faker/test/test_determinism.rb:13:in `test_determinism'

@felipesousafs please fix it and let us know when it's ready for review again. Thanks!

@vbrazo
Copy link
Member

vbrazo commented May 18, 2018

After making the changes, could you also update the CHANGELOG.md and add the title of this PR + your GitHub ID?

[UPDATES]: Remove puts from test file and fix the quote method
@coveralls
Copy link

coveralls commented May 19, 2018

Pull Request Test Coverage Report for Build 2171

  • 13 of 13 (100.0%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 99.65%

Files with Coverage Reduction New Missed Lines %
lib/faker/time.rb 7 100.0%
Totals Coverage Status
Change from base Build 2135: 0.02%
Covered Lines: 2278
Relevant Lines: 2286

💛 - Coveralls

@faker-ruby faker-ruby deleted a comment from coveralls May 19, 2018
@vbrazo
Copy link
Member

vbrazo commented May 19, 2018

You still need to fix the rubocop violations. Please just run bundle exec rubocop -a and then we should be fine. @felipesousafs

@vbrazo
Copy link
Member

vbrazo commented May 19, 2018

Let's get this PR merged today. Lemme know when you fix the violations.

@vbrazo
Copy link
Member

vbrazo commented May 19, 2018

Travis is still red because of Rubocop 😢

@felipesousafs
Copy link
Contributor Author

test/test_faker_dota.rb:6:15: C: Style/PercentLiteralDelimiters: %w-literals should be delimited by [ and ]. @heroes = %w(abaddon alchemist axe beastmaster brewmaster bristleback centaur ... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ test/test_faker_dota.rb:7:16: C: Layout/AlignArray: Align the elements of an array literal if they span more than one line. chaos_knight clockwerk doom dragon_knight earth_spirit earthshaker ^^^^^^^^^^^^

What does this means and how can I fix this?
(I'm new with this 😄 )

@vbrazo
Copy link
Member

vbrazo commented May 19, 2018

No worries. You should use Rubocop in your project because it'll force you to follow a few great Ruby practices. Lemme take a look at your question.

@vbrazo
Copy link
Member

vbrazo commented May 19, 2018

I basically pulled your branch and ran bundle exec rubocop -a and fixed them. Take a look at the Rubocop repository to understand more about this gem 👍

@vbrazo vbrazo merged commit 4accbb7 into faker-ruby:master May 19, 2018
@felipesousafs
Copy link
Contributor Author

Nice!
Thanks for the tips, man.

@vbrazo vbrazo changed the title [ADD] Dota2: Str Heroes, heroes quotes, Items, Teams, Players Dota2 API: Str Heroes, heroes quotes, Items, Teams, Players May 19, 2018
@vbrazo vbrazo changed the title Dota2 API: Str Heroes, heroes quotes, Items, Teams, Players Dota API: Str Heroes, heroes quotes, Items, Teams, Players May 20, 2018
@vbrazo vbrazo self-requested a review July 19, 2018 01:27
davidmorton0 pushed a commit to davidmorton0/faker that referenced this pull request Jul 12, 2021
…ruby#1174)

* [ADD] Dota2: Str Heroes, heroes quotes, Items, Teams, Players

* Restore rake task to match with master

* [ADD] Dota2: Str Heroes, heroes quotes, Items, Teams, Players
[UPDATES]: Remove puts from test file and fix the quote method

* [UPDATE]: Change the type of the array of strings used.

* Update test_faker_dota.rb
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants