Pure autocorrect #152
Merged
Pure autocorrect #152
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
Where auto-correct is available, of course. Some manual touches of file_signature to resolve cherry-pick conflicts
|
If this gets merged, I'll probably close #137 since there I drifted off into trying to rework test that were previously insufficiently differentiated. |
SaravShah
approved these changes
Apr 17, 2018
jmartin-sul
requested changes
Apr 18, 2018
| other_checksums = other.checksums | ||
| matching_keys = self_checksums.keys & other_checksums.keys | ||
| return false if matching_keys.size == 0 |
There was a problem hiding this comment.
the intent according to this comment:
27db283#diff-11a0d7906801d9dea0eccb85667ad811R75
seemed to be to keep #size == 0 instead of switching to #empty, for compatibility's sake?
There was a problem hiding this comment.
It only matters when called on Pathname objects, which don't have empty? on old rubies. Here, it is called on an Array, and that's fine.
| group_addtions.add_file_instance(file.signature, file.instances[0]) | ||
| end | ||
| end | ||
| version_additions.groups << group_addtions if group_addtions.files.size > 0 | ||
| version_additions.groups << group_addtions unless group_addtions.files.empty? |
There was a problem hiding this comment.
ditto the compatibility question from the prior comment
| @@ -124,7 +128,7 @@ def generate_content_metadata(file_group, object_id, version_id) | |||
| # @return [Boolean] True if contentMetadata has essential file attributes, else raise exception | |||
| def validate_content_metadata(content_metadata) | |||
| result = validate_content_metadata_details(content_metadata) | |||
| raise Moab::InvalidMetadataException, result[0] + " ..." if result.size > 0 | |||
| raise Moab::InvalidMetadataException, result[0] + " ..." unless result.empty? | |||
There was a problem hiding this comment.
ditto empty? (and a few more below, won't mark the rest from here out)
jmartin-sul
approved these changes
Apr 18, 2018
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.
Cherry-picked from #137. This should be easy to digest as (almost) entirely a bunch of repetitive changes. With the exception of one file (resolving conflicts), all the code changes were done mechanically via rubocop based on
.rubocop_todo.yml.If a particular style enforced by the tool is objectionable, then IMO, the proper reaction is to add exceptions or configuration to
.rubocop.yml. It is counterproductive to leave "offenses" listed in aTODOfile documented as:The point is for the user to remove these configuration records one by one as the offenses are removed from the code base.