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

Add basic coverage to RailsStats::CodeStatistics#to_s #10

Merged
merged 7 commits into from
Oct 15, 2020
Merged

Add basic coverage to RailsStats::CodeStatistics#to_s #10

merged 7 commits into from
Oct 15, 2020

Conversation

etagwerker
Copy link
Member

@etagwerker etagwerker commented Oct 7, 2020

Hey @bleonard,

This PR adds minitest to the project and it adds a simple scenario for RailsStats::CodeStatistics#to_s

I added a Rails 6.0 application in test/dummy to make it easier to test rails_stats. I didn't want to add a lot of tests before getting your feedback. Do you think this is a step in the right direction?

This is what it should look like in your test environment:

Screen Shot 2020-10-06 at 11 48 25 PM

Please let me know.

Thanks!

@bleonard
Copy link
Contributor

bleonard commented Oct 7, 2020

That's certainly a bunch of files, but guess they are all in the test directory.
Feel free to give it a shot if you want. I didn't even know people were using this project :-)

@etagwerker
Copy link
Member Author

@bleonard Cool, thanks! Do you think this is ready to be merged? I just solved the conflict. 😄

@etagwerker
Copy link
Member Author

@bleonard One more thing, could you enable Travis CI for this project? Or do you prefer checks with GitHub Actions?

Copy link

@arielj arielj left a comment

Choose a reason for hiding this comment

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

It would be nice to have a section in the README explaining how to run the tests of the gem.

@@ -1 +1,12 @@
require "rails_stats"
Copy link

Choose a reason for hiding this comment

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

Removing this lines makes the rake command unable to find the stats task.

require "minitest/pride"
require "minitest/around/spec"

require "rails_stats/all"
Copy link

Choose a reason for hiding this comment

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

Just nit picking here, but it's warning about a missing trailing new line

@etagwerker
Copy link
Member Author

etagwerker commented Oct 15, 2020

@arielj Thanks for reviewing this PR! Just addressed your comments 👍

Copy link

@arielj arielj left a comment

Choose a reason for hiding this comment

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

Looks good!

@etagwerker etagwerker merged commit a6e059c into fastruby:master Oct 15, 2020
@etagwerker etagwerker deleted the add-basic-coverage branch October 15, 2020 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants