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

Add an array level in JSON output to repInfo #728

Merged
merged 3 commits into from
Apr 12, 2022

Conversation

tledoux
Copy link
Contributor

@tledoux tledoux commented Apr 10, 2022

in order to support a list of files to be validated

Fixes #667

@codecov
Copy link

codecov bot commented Apr 10, 2022

Codecov Report

Merging #728 (439b145) into integration (3c48e70) will increase coverage by 0.77%.
The diff coverage is 25.42%.

@@                Coverage Diff                @@
##             integration     #728      +/-   ##
=================================================
+ Coverage          45.68%   46.46%   +0.77%     
- Complexity          1049     1054       +5     
=================================================
  Files                 57       57              
  Lines               9147     9059      -88     
  Branches            1684     1606      -78     
=================================================
+ Hits                4179     4209      +30     
+ Misses              4410     4308     -102     
+ Partials             558      542      -16     
Impacted Files Coverage Δ
...edu/harvard/hul/ois/jhove/handler/JsonHandler.java 24.94% <25.42%> (+3.77%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53749ca...439b145. Read the comment docs.

@carlwilson
Copy link
Member

Thanks, @tledoux I was going to look at this myself as one of a couple of PRs of my own, but I hadn't done it yet so you've saved me some time. Will take a look shortly.

@carlwilson carlwilson self-requested a review April 11, 2022 10:14
@carlwilson carlwilson added the bug A product defect that needs fixing label Apr 11, 2022
@carlwilson carlwilson added this to the JHOVE 1.26 milestone Apr 11, 2022
@tledoux
Copy link
Contributor Author

tledoux commented Apr 11, 2022

Hi @carlwilson , most welcome to help you on this.
The correction itself was just adding an array level to handle the repInfo in case more than one is emitted (I added a test for this case).
I will stop with this last commit trying to decrease the complexity to make QA happier, but not enough ;-)

@carlwilson
Copy link
Member

Thanks once again @tledoux. There are plenty of overcomplex methods in the code base that have been there for a long time. In many cases it's not a new problem, you get blamed simply for touching a function that was already overlong. TLDR, don't worry too much about the QA :)

Copy link
Member

@carlwilson carlwilson left a comment

Choose a reason for hiding this comment

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

Approved subject to test results 👍

@carlwilson carlwilson merged commit cf82291 into openpreserve:integration Apr 12, 2022
@tledoux tledoux deleted the jsonMultiRepInfo branch January 10, 2023 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A product defect that needs fixing
Projects
None yet
2 participants