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

Reactivate coveralls #1571

Open
wants to merge 2 commits into
base: staging
Choose a base branch
from
Open

Reactivate coveralls #1571

wants to merge 2 commits into from

Conversation

eugenk
Copy link
Member

@eugenk eugenk commented Jan 15, 2016

This is supposed to reactivate coveralls notifications in pull requests.

The branch of this pull request is not based on staging, but on the one of #1572 (update_gems). Reviewing that one first may be easier.

The first commit of this branch is
Add coveralls gem (and resolve dependencies). 30abe0d

@@ -2,6 +2,9 @@
ENV["RAILS_ENV"] ||= 'test'

require 'simplecov'
require 'coveralls'
Coveralls.wear_merged!('rails')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Copy link
Member Author

Choose a reason for hiding this comment

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

No thanks. You should stop suggesting this as soon as #1570 gets merged.

@eugenk
Copy link
Member Author

eugenk commented Jan 15, 2016

Okay, it seems like this needs to be stalled until we upgrade rails to version 4.

config.x_frame_options = 'DENY'
config.x_content_type_options = "nosniff"
config.x_xss_protection = {:value => 1, :mode => false}
config.x_xss_protection = 'value=1; mode=false'

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Copy link
Member Author

Choose a reason for hiding this comment

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

No thanks. You should stop suggesting this as soon as #1570 gets merged.

@eugenk eugenk removed the stalled label Jan 17, 2016
@eugenk
Copy link
Member Author

eugenk commented Jan 17, 2016

It seems like we can do this anyway. The error (not visible here any more) was caused by the gem bootstrap-sass and not by coveralls.

config.x_content_type_options = "nosniff"
config.x_xss_protection = {:value => 1, :mode => false}
config.x_content_type_options = 'nosniff'
config.x_xss_protection = 'value=1; mode=false'

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

Copy link
Member Author

Choose a reason for hiding this comment

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

No thanks. You should stop suggesting this as soon as #1570 gets merged.

@eugenk eugenk force-pushed the reactivate_coveralls branch 4 times, most recently from 0b992eb to 81e052e Compare January 18, 2016 08:39
@ebolloff ebolloff self-assigned this Jan 18, 2016
@eugenk eugenk force-pushed the reactivate_coveralls branch 2 times, most recently from d0884d2 to 2fab6fc Compare January 18, 2016 14:41
@eugenk
Copy link
Member Author

eugenk commented Jan 18, 2016

I rebased this on staging.

@eugenk
Copy link
Member Author

eugenk commented Jan 18, 2016

For an unknown reason, blank lines and lines only containing end are considered untested by coveralls.
See, for instance, https://coveralls.io/builds/4763366/source?filename=lib%2Fproof_execution.rb

@ebolloff
Copy link
Member

This seems to be an error in simplecov, someone already reported it. Should we wait for it to be merged?

@eugenk
Copy link
Member Author

eugenk commented Jan 20, 2016

Yes, let's wait for simplecov-ruby/simplecov#444 and then update the gem.

@eugenk eugenk added the stalled label Jan 20, 2016
@eugenk
Copy link
Member Author

eugenk commented Apr 28, 2016

We will get back as soon as there is a new release of simplecov (> 0.11.2)

@ebolloff
Copy link
Member

We have simplecov-0.12.0 now, so let's give this another try.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 0ef6a45 on reactivate_coveralls into * on staging*.

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