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

Silence @track_files_glob ivar warning #447

Merged
merged 1 commit into from
Jan 25, 2016

Conversation

craiglittle
Copy link
Contributor

Fixes #446.

@colszowka
Copy link
Collaborator

The build failed due to #448 apparently, we'll have to get that one out of the door first.

We've been dealing with those pesky ruby warnings basically since the beginning of the project, thanks for taking the time to take this one out!

In order to keep things consistent, could you please change the implementation to what we're doing for all the other instance vars in the configuration class? See for example https://github.com/craiglittle/simplecov/blob/fix-ivar-warning/lib/simplecov/configuration.rb#L36-L37, basically we return if the var is defined and truthy, otherwise do the assignment. I would prefer not to introduce another pattern for dealing with these things here. It's arguable though whether your solution isn't nicer/cleaner - if you prefer I'd also be good if we changed the existing methods to use your style of pre-initializing on extend.

@craiglittle
Copy link
Contributor Author

Sure, I'll change the other methods to pre-initialize the instance variables in the extended hook. FWIW, I leaned this direction because of @xaviershay's comment in the related issue.

@craiglittle
Copy link
Contributor Author

Okay, I decided to go the other direction and switch the track_files_glob implementation to mirror the other methods because doing otherwise would change the semantics of the other methods.

Once we get the RuboCop PR squared away, I'll rebase this onto latest master.

@colszowka
Copy link
Collaborator

Perfect, thank you!

@xaviershay
Copy link
Collaborator

apologies for the bad intel, I didn't realise we had prior art!

@colszowka colszowka mentioned this pull request Jan 22, 2016
@colszowka
Copy link
Collaborator

It's not the best style, but this build failed solely because of Rubycop, which was fixed in #451, so I'm merging this to avoid having to make another roundtrip with @craiglittle for rebasing this against master

colszowka added a commit that referenced this pull request Jan 25, 2016
Silence `@track_files_glob` ivar warning
@colszowka colszowka merged commit 058dbba into simplecov-ruby:master Jan 25, 2016
colszowka added a commit that referenced this pull request Jan 25, 2016
@craiglittle craiglittle deleted the fix-ivar-warning branch January 25, 2016 17:55
@craiglittle
Copy link
Contributor Author

Do you think a release will be cut soon? It'd be swell to get this warning out of my build. :)

@colszowka
Copy link
Collaborator

@craiglittle Yes, it's out in 0.11.2 right now

@craiglittle
Copy link
Contributor Author

Awesome, much appreciated! 😄

craiglittle pushed a commit to craiglittle/simplecov that referenced this pull request Feb 13, 2016
craiglittle added a commit to craiglittle/simplecov that referenced this pull request Feb 13, 2016
Fixes simplecov-ruby#462.

The implementation of `track_files` was refactored recently to suppress
a warning in the underlying instance variable had not been initialized.
This had the effect of changing the behavior when trying to nullify the
default configuration of tracked files in some cases through the use of
nil.

With this change, a declaration of `track_files nil` will once again
clear out any previous configuration and essentially disable the
feature.

Reference:
  * simplecov-ruby#447
  * simplecov-ruby#462
craiglittle added a commit to craiglittle/simplecov that referenced this pull request Feb 13, 2016
Fixes simplecov-ruby#462.

The implementation of `track_files` was refactored recently to suppress
a warning in the underlying instance variable had not been initialized.
This had the effect of changing the behavior when trying to nullify the
default configuration of tracked files in some cases through the use of
nil.

With this change, a declaration of `track_files nil` will once again
clear out any previous configuration and essentially disable the
feature.

Reference:
  * simplecov-ruby#447
  * simplecov-ruby#462
craiglittle added a commit to craiglittle/simplecov that referenced this pull request Feb 13, 2016
Fixes simplecov-ruby#462.

The implementation of `track_files` was refactored recently to suppress
a warning in the underlying instance variable had not been initialized.
This had the effect of changing the behavior when trying to nullify the
default configuration of tracked files in some cases through the use of
nil.

With this change, a declaration of `track_files nil` will once again
clear out any previous configuration and essentially disable the
feature.

Reference:
  * simplecov-ruby#447
  * simplecov-ruby#462
craiglittle added a commit to craiglittle/simplecov that referenced this pull request Feb 13, 2016
Fixes simplecov-ruby#462.

The implementation of `track_files` was refactored recently to suppress
a warning in the underlying instance variable had not been initialized.
This had the effect of changing the behavior when trying to nullify the
default configuration of tracked files in some cases through the use of
nil.

With this change, a declaration of `track_files nil` will once again
clear out any previous configuration and essentially disable the
feature.

Reference:
  * simplecov-ruby#447
  * simplecov-ruby#462
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Feb 17, 2016
## Enhancements

## Bugfixes

0.11.2 2016-02-03 ([changes](simplecov-ruby/simplecov@v0.11.1...v0.11.2))
=================

## Enhancements

* Do not globally pollute Array and Hash with `merge_resultset` utility methods. See [#449](simplecov-ruby/simplecov#449) (thanks @amatsuda)
* Do not `mkdir_p` the `coverage_path` on every access of the method (See [#453](simplecov-ruby/simplecov#453) (thanks @paddor)
* Fixes a Ruby warning related to the `track_files` configuration. See [#447](simplecov-ruby/simplecov#447) (thanks @craiglittle)
* Add a group for background jobs to default Rails profile. See [#442](simplecov-ruby/simplecov#442) (thanks @stve)

## Bugfixes

* Fix root_filter evaluates SimpleCov.root before initialization. See [#437](simplecov-ruby/simplecov#437) (thanks @tmtm)
PragTob pushed a commit that referenced this pull request Feb 4, 2017
Fixes #462.

The implementation of `track_files` was refactored recently to suppress
a warning in the underlying instance variable had not been initialized.
This had the effect of changing the behavior when trying to nullify the
default configuration of tracked files in some cases through the use of
nil.

With this change, a declaration of `track_files nil` will once again
clear out any previous configuration and essentially disable the
feature.

Reference:
  * #447
  * #462
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

3 participants