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

Address some Ruby warnings #1995

Merged
merged 5 commits into from
Feb 25, 2020
Merged

Address some Ruby warnings #1995

merged 5 commits into from
Feb 25, 2020

Conversation

nbeyer
Copy link
Contributor

@nbeyer nbeyer commented Feb 19, 2020

  • Add warnings enablement to spec_helper.rb to see future warnings
  • Use instance_variable_defined? to clean up not initialized warnings
  • Add 'inherited' to NON_OVERRIDABLE list
  • Add multi_xml to test dependencies in Gemfile to address spec failure

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

This looks good, needs a rebase please.

CHANGELOG.md Outdated Show resolved Hide resolved
* Add warnings enablement to spec_helper.rb to see future warnings
* Use instance_variable_defined? to clean up not initialized warnings
* Add 'inherited' to NON_OVERRIDABLE list
* Add multi_xml to test dependencies in Gemfile to address spec failure
@nbeyer
Copy link
Contributor Author

nbeyer commented Feb 24, 2020

@dblock This PR has been rebased.

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

The multi_xml thing looks suspicious. Otherwise good to go.

Gemfile Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@dblock dblock merged commit 53d826f into ruby-grape:master Feb 25, 2020
@dblock
Copy link
Member

dblock commented Feb 25, 2020

Merged, thank you.

@dblock
Copy link
Member

dblock commented Feb 25, 2020

Now that we run with --warnings, how far are we from making tests fail on unexpected warnings?

@nbeyer nbeyer deleted the undefined_warnings branch February 25, 2020 15:56
@nbeyer
Copy link
Contributor Author

nbeyer commented Feb 25, 2020

I've not found anything great for failing tests based on unexpected warnings. It's possible to plug into Ruby's Warning module, but it would require some framework building, as it seems likely that warnings from dependencies are going to pop-up, which would be out of the control of this project.

basjanssen pushed a commit to basjanssen/grape that referenced this pull request Feb 28, 2020
* Address some Ruby warnings

* Add warnings enablement to spec_helper.rb to see future warnings
* Use instance_variable_defined? to clean up not initialized warnings
* Add 'inherited' to NON_OVERRIDABLE list
* Add multi_xml to test dependencies in Gemfile to address spec failure

* Add changelog entry

* expand changelog entry

* adjust changelog

* remove unnecessary multi_xml
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.

2 participants