Fix warnings #1067
Merged
Fix warnings #1067
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -1,4 +1,4 @@ | |||
| require "set" | |||
| require 'set' | |||
|
@joaomdmoura nu? You want to merge it? 😺 |
|
Still need to review it 😁 But it's an open tab already ahha |
| autoload :FlattenJson | ||
| autoload :Null | ||
| autoload :JsonApi | ||
| autoload :FragmentCache |
|
this is nice just a couple of lines that I wanted to know more about if possible 😄 |
|
btw, can you squash the commit withe the failing test please? |
|
rebased |
|
Sorry, I think you might need to rebase it again 😁 test failing btw 😄 |
|
Rebased and force pushed again |
JRuby-specific: fix 'warning: (...) interpreted as grouped expression'
|
LGTM, merging it. |
| $VERBOSE = verbose | ||
| end | ||
|
|
||
| end |
There was a problem hiding this comment.
required because of a Ruby bug or as a 'safety valve' for unfixable or not-worth-fixing warnings. such is life
e.g.
private
ActiveModelSerializers.silence_warnings do
attr_reader :resource, :adapter_opts, :serializer_opts
end
| # Ensure backward compatibility with Minitest 4 | ||
| Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test) | ||
|
|
||
| require "capture_warnings" | ||
| @capture_warnings = CaptureWarnings.new(fail_build = false) |
There was a problem hiding this comment.
factored these out into test/support/*.rb so it's easier to see what's being configured
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Aug 28, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067
9 tasks
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Sep 4, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Sep 6, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067 Style note: To make diffs on the adapters smalleer and easier to read, I've maintained the same identention that was in the original file. I've decided to prefer ease of reading the diff over style, esp. since we may later return to the preferred class declaration style.
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Sep 9, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067 Style note: To make diffs on the adapters smalleer and easier to read, I've maintained the same identention that was in the original file. I've decided to prefer ease of reading the diff over style, esp. since we may later return to the preferred class declaration style.
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Sep 9, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067 Style note: To make diffs on the adapters smalleer and easier to read, I've maintained the same identention that was in the original file. I've decided to prefer ease of reading the diff over style, esp. since we may later return to the preferred class declaration style.
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Sep 9, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067 Style note: To make diffs on the adapters smalleer and easier to read, I've maintained the same identention that was in the original file. I've decided to prefer ease of reading the diff over style, esp. since we may later return to the preferred class declaration style. with '#' will be ignored, and an empty message aborts the commit.
bf4
added a commit
to bf4/active_model_serializers
that referenced
this pull request
Sep 9, 2015
I was seeing transient failures where adapters may not be registered. e.g. https://travis-ci.org/rails-api/active_model_serializers/builds/77735382 Since we're using the Adapter, JsonApi, and Json classes as namespaces, some of the conventions we use for modules don't apply. Basically, we don't want to define the class anywhere besides itself. Otherwise, the inherited hooks may not run, and some adapters may not be registered. For example: If we have a class Api `class Api; end` And Api is also used as a namespace for `Api::Product` And the classes are defined in different files. In one file: ```ruby class Api autoload :Product def self.inherited(subclass) puts p [:inherited, subclass.name] puts end end ``` And in another: ```ruby class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` If we load the Api class file first, the inherited hook will be defined on the class so that when we load the Api::Product class, we'll see the output: ```plain [ :inherited, Api::Product] ``` However, if we load the Api::Product class first, since it defines the `Api` class and then inherited from it, the Api file was never loaded, the hook never defined, and thus never run. By defining the class as `class Api::Product < Api` We ensure the the Api class MUST be defined, and thus, the hook will be defined and run and so sunshine and unicorns. Appendix: The below would work, but triggers a circular reference warning. It's also not recommended to mix require with autoload. ```ruby require 'api' class Api class Product < Api def sell_sell_sell! # TODO: sell end end end ``` This failure scenario was introduced by removing the circular reference warnings in rails-api#1067 Style note: To make diffs on the adapters smalleer and easier to read, I've maintained the same identention that was in the original file. I've decided to prefer ease of reading the diff over style, esp. since we may later return to the preferred class declaration style. with '#' will be ignored, and an empty message aborts the commit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'm open to changing what's in this PR and its formatting. The main thing is, builds will now fail with warnings.