Move and deprecate trait methods from FactoryBot #1285
Merged
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
houndci-bot
reviewed
Apr 19, 2019
aledustet
force-pushed
the
chore/move-trait-methods-to-factory-bot-internal
branch
from
4133856
to
0910267
Compare
spec/support/macros/deprecation.rb
Outdated
| @@ -13,6 +13,8 @@ def silence_deprecation(example) | |||
| config.include SilenceDeprecation | |||
|
|
|||
| config.around :example, silence_deprecation: true do |example| | |||
| silence_deprecation(example) | |||
| with_silenced_output do | |||
There was a problem hiding this comment.
I am not a fan of this with_silenced_output method. This is not longer just silencing the deprecation, it is silencing any output, which would make things hard to debug if anything went wrong.
Instead, I think we can just update the silence_deprecation method to set FactoryBot::DEPRECATOR.silenced instead of ActiveSupport::Deprecation.silenced.
There was a problem hiding this comment.
We might also consider renaming FactoryBot::DEPRECATOR to FactoryBot::Deprecation so it better matches ActiveSupport::Deprecation.
aledustet
force-pushed
the
chore/move-trait-methods-to-factory-bot-internal
branch
from
0910267
to
c393e7c
Compare
composerinteralia
approved these changes
Apr 26, 2019
aledustet
force-pushed
the
chore/move-trait-methods-to-factory-bot-internal
branch
from
5c4b9d9
to
d7a2b72
Compare
Why: These are essentially internal methods that should not be publicly available from the base namespace. One thing worth noticing is that the use of this methods internally was almost exclusively in the `syntax/default` except for one use on the `factory_bot/definition`. Also the deprecation silencing module was referring to the singleton instance of the ```ActiveRecord::Deprecation``` class and not to the new deprecation instance that was being used in the ```FactoryBot``` module. This PR: - Moves the `trait_by_name` and `register_trait` into the `FactoryBot::Internal` module - Deprecates uses of `trait_by_name`, `register_trait` and `traits` from the `FactoryBot` module. - Rename DEPRECATOR => Deprecation This is one of the steps towards fixing [this issue](#1281)
aledustet
force-pushed
the
chore/move-trait-methods-to-factory-bot-internal
branch
from
d7a2b72
to
b74acf4
Compare
aledustet
added a commit
that referenced
this pull request
Apr 26, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` from the ```FactoryBot``` module. This is one of the steps towards fixing [this issue](#1285 (comment))
aledustet
added a commit
that referenced
this pull request
Apr 26, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` from the ```FactoryBot``` module. This is one of the steps towards fixing [this issue](#1285 (comment))
aledustet
added a commit
that referenced
this pull request
May 3, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. This is one of the steps towards fixing [this issue](#1285 (comment))
aledustet
added a commit
that referenced
this pull request
May 24, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. This is one of the steps towards fixing [this issue](#1285 (comment))
aledustet
added a commit
that referenced
this pull request
Jun 4, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. - Refactor rewind sequences test to use spies This is one of the steps towards fixing [this issue](#1285 (comment))
aledustet
added a commit
that referenced
this pull request
Jun 4, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. - Refactor rewind sequences test to use spies This is one of the steps towards fixing [this issue](#1285 (comment))
aledustet
added a commit
that referenced
this pull request
Jun 4, 2019
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. This is one of the steps towards fixing [this issue](#1285 (comment))
connorchris831
pushed a commit
to connorchris831/factory_bot
that referenced
this pull request
Dec 13, 2022
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. - Refactor rewind sequences test to use spies This is one of the steps towards fixing [this issue](thoughtbot/factory_bot#1285 (comment))
SriHarshaxi
pushed a commit
to SriHarshaxi/factory_bot
that referenced
this pull request
Mar 17, 2023
Why: Another run of internal methods that should not be publicly available from the base namespace. This time the sequence involving methods were moved. It's worth noticing that the ```Internal``` class is getting crowded. Maybe we can start name-spacing the internal groups into modules under ```internal/sequence.rb``` ```internal/trait.rb``` and so on. Thoughts? This PR: - Moves the ```register_sequence```, ```sequence_by_name```, ```sequences``` and ```rewind_sequences``` to the ```FactoryBot::Internal``` module. - Deprecates uses of ```sequence_by_name```, and ```sequences``` from the ```FactoryBot``` module. - Refactor rewind sequences test to use spies This is one of the steps towards fixing [this issue](thoughtbot/factory_bot#1285 (comment))
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.
Why:
These are essentially internal methods that should not be publicly
available from the base namespace.
One thing worth noticing is that the use of these methods internally was
almost exclusively in the
syntax/defaultexcept for one use on thefactory_bot/definition.Also, the deprecation silencing module was referring to the singleton
an instance of the
ActiveRecord:: Deprecationclass and not to the newdeprecation instance that was being used in the
FactoryBotmodule.This PR:
trait_by_nameandregister_traitinto theFactoryBot::Internalmoduletrait_by_name,register_traitandtraitsfromthe
FactoryBotmodule.This is one of the steps towards fixing this
issue