Change HashDiff Constant to Hashdiff #65
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
|
Test failures seem to be Rubocop issues that existed before this PR. |
jfelchner
changed the title
|
Thanks a lot @jfelchner . WDYT @krzysiek1507 ? |
krzysiek1507
requested changes
May 27, 2019
luke-hill
reviewed
May 28, 2019
Why This Change Is Necessary ======================================================================== The name of this gem is `hashdiff`. There is another gem named `hash_diff`. When they are both required via dependencies, errors are thrown. Additionally since the behavior is different, gems that are relying on `hashdiff`'s behavior, may get `hash_diff`'s behavior and vice versa. The reason for the conflict is because, based on ruby conventions, _'s are the separators for camel case. So: ```ruby 'hashdiff'.camelize # => "Hashdiff" 'hash_diff'.camelize # => "HashDiff" ``` Unfortunately instead of making the base module of this gem `Hashdiff`, it's called `HashDiff` and it conflicts. How These Changes Address the Issue ======================================================================== We change all of the constants to the new constant, including in documentation and tests. Then we add a shim to allow the old constant to be used and therefore maintain backward compatibility. Finally we add a warning message for users of the gem that the old constant will be removed in the next major version bump. Side Effects Caused By This Change ======================================================================== A new warning is added when loading the gem.
Why This Change Is Necessary ======================================================================== `require_relative` is error-prone in certain situations, specifically in Ruby < 2.4 and should never be used for requires inside of gems. How These Changes Address the Issue ======================================================================== Revert the change from `require` to `require_relative`. Side Effects Caused By This Change ======================================================================== Fewer bugs.
luke-hill
approved these changes
May 28, 2019
|
@liufengyun just remember this needs to be version |
|
Thank a lot @jfelchner 🎉 And thanks for reviewing @krzysiek1507 @luke-hill ! |
|
@liufengyun Could you please update the GitHub repository description as well?
|
|
@bquorning It's changed now, thanks for the catch. |
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 This Change Is Necessary
The name of this gem is
hashdiff. There is another gem namedhash_diff. When they are both required via dependencies, errors arethrown. Additionally since the behavior is different, gems that are
relying on
hashdiff's behavior, may gethash_diff's behavior andvice versa.
The reason for the conflict is because, based on ruby conventions, _'s
are the separators for camel case.
So:
Unfortunately instead of making the base module of this gem
Hashdiff,it's called
HashDiffand it conflicts.How These Changes Address the Issue
We change all of the constants to the new constant, including in
documentation and tests.
Then we add a shim to allow the old constant to be used and therefore
maintain backward compatibility.
Finally we add a warning message for users of the gem that the old
constant will be removed in the next major version bump.
Side Effects Caused By This Change
A new warning is added when loading the gem.
References:
hash_diff#45