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

Attempt to install codecov #543

Merged
merged 10 commits into from
Apr 18, 2019
Merged

Attempt to install codecov #543

merged 10 commits into from
Apr 18, 2019

Conversation

jywarren
Copy link
Member

@codeclimate
Copy link

codeclimate bot commented Apr 16, 2019

Code Climate has analyzed commit d1c116f and detected 0 issues on this pull request.

View more on Code Climate.

@kaustubh-nair
Copy link
Member

kaustubh-nair commented Apr 16, 2019

Are we switching to codecov instead of coveralls?
This would be great! We wouldn't have security issues because of making the token public for coveralls

@jywarren
Copy link
Member Author

jywarren commented Apr 16, 2019 via email

@jywarren
Copy link
Member Author

I can't get it to make comments like it did on image sequencer...

@jywarren
Copy link
Member Author

Oh I guess I need to install a gem or something?

@jywarren
Copy link
Member Author

Working from publiclab/image-sequencer#1014

@kaustubh-nair
Copy link
Member

Looks like you forgot to modify the travis file

@kaustubh-nair
Copy link
Member

Oh you just did 😄

@kaustubh-nair
Copy link
Member

Travis logs say,
--> No coverage report found.
Looks like we need to generate the report before uploading it to Codecov.
Should we try installing the Simplecov gem?
It should generate the reports for us.

@alaxalves
Copy link
Member

alaxalves commented Apr 16, 2019

Travis logs say,
--> No coverage report found.
Looks like we need to generate the report before uploading it to Codecov.
Should we try installing the Simplecov gem?
It should generate the reports for us.

@jywarren @kaustubh-nair Yes, in order to get this working we need to install simplecov gem and set it up in our test_helper.rb file as in here. Also, if we get CodeCov and SimpleCov properly set up here we could easily do what I have suggested in #495 (comment) FYK this example displayed is set using CodeCov/SimpleCov.

@kaustubh-nair
Copy link
Member

Okay, let's do it

Gemfile Outdated
@@ -41,6 +41,7 @@ end

group :test do
gem 'test-unit'
gem "simplecov", require: false
Copy link

Choose a reason for hiding this comment

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

Gems should be sorted in an alphabetical order within their section of the Gemfile. Gem simplecov should appear before test-unit.

Gemfile Outdated
@@ -41,6 +41,7 @@ end

group :test do
gem 'test-unit'
gem "simplecov", require: false
Copy link

Choose a reason for hiding this comment

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

Prefer single-quoted strings when you don't need string interpolation or special symbols.

@kaustubh-nair
Copy link
Member

Rubocop is being really strict :/
We should add a config file and remove whatever config options we don't need

@alaxalves
Copy link
Member

alaxalves commented Apr 17, 2019

Rubocop is being really strict :/
We should add a config file and remove whatever config options we don't need

I got a sample yml I used in previous projects, I'll MR it and we could discuss its rules we'd want. Done this in #547

@alaxalves alaxalves mentioned this pull request Apr 17, 2019
5 tasks
@jywarren
Copy link
Member Author

jywarren commented Apr 17, 2019 via email

@kaustubh-nair
Copy link
Member

I added the gem but it still doesn't seem to be sending the reports
Any idea what's wrong here?

@codecov
Copy link

codecov bot commented Apr 18, 2019

Codecov Report

❗ No coverage uploaded for pull request base (main@0a8813c). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##             main    #543   +/-   ##
======================================
  Coverage        ?   69.4%           
======================================
  Files           ?      31           
  Lines           ?    1242           
  Branches        ?       0           
======================================
  Hits            ?     862           
  Misses          ?     380           
  Partials        ?       0

@kaustubh-nair
Copy link
Member

yay! 😄

@kaustubh-nair
Copy link
Member

The coverage report was not accessible from inside the docker container. So I created a shared volume to store the report there.

@kaustubh-nair
Copy link
Member

It seems the docker volume was not necessary after all..
Anyway, @jywarren this is ready for merge! 😃

@jywarren
Copy link
Member Author

Oooh what a pretty graph! Merging this!!!!!

@jywarren jywarren merged commit 7c1d1b8 into main Apr 18, 2019
@jywarren
Copy link
Member Author

@cesswairimu check it out! 👀

@jywarren
Copy link
Member Author

@publiclab/plots2-reviewers @publiclab/reviewers would we like this to be installed on plots2 as well?

@cesswairimu
Copy link
Collaborator

Wow this looks amazing @jywarren @kaustubh-nair 🎉 🎉 . How do I get it to comment on my PR just rebased but can't see the bot 👀

@cesswairimu
Copy link
Collaborator

My bad, I see the bot now 😃

chen-robert pushed a commit to chen-robert/mapknitter that referenced this pull request Dec 5, 2019
* Attempt to install codecov

* Create .codecov.yml

* Update .travis.yml

* Add simplecov

* Add docker volume and change formatter

* Remove unneccessary files and fix rubocop issues

* Remove coveralls and add more filters

* modify gitignore

* add .simplecov file

* remove docker volume
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.

None yet

4 participants