Closed alexdunnjpl closed 1 year ago
converting back to draft to ensure that outstanding minor items in OP aren't overlooked when I come back to this next week
@tloubrieu-jpl ready for review
@jordanpadams see https://github.com/NASA-PDS/registry-sweepers/pull/10/commits/95489170c96467974272a813e641d3047f04c37a for implementation and test-cases for support of alternative_ids
@alexdunnjpl I removed the sprint-backlog tag from the PR, since this is bugging me in the zenhub board.
Mostly not critical comments from me, but sonatype and Sean's comments require some changes .
@tloubrieu-jpl what of Sean's requires further action?
Regarding Sonatype, are you aware of a way to trigger a fresh sweep of a PR? I think possibly the force pushes are confusing it. Or are you just talking about the one issue mentioned in your recent comment?
@tloubrieu-jpl regarding the outstanding sonatype error, ~it's a false positive - not picked up by mypy either.~ actually it's stale - that line is correctly annotated since 6c38aa5
I've removed some obsolete mypy ignores, but otherwise it should be good to go.
Just realised there's one outstanding question for @jordanpadams. What exactly do you want the metadata keys to be?
Currently it's ops:Provenance/ops:parent_collection_identifiers
and ops:Provenance/ops:parent_bundle_identifiers
, but the pluralisation, while arguably correct, may be at odds with user expectations given fields like pds:Internal_Reference/pds:lid_reference
which are collections but whose field name is singular.
@alexdunnjpl let's go with the singular
Mostly not critical comments from me, but sonatype and Sean's comments require some changes .
@tloubrieu-jpl what of Sean's requires further action?
@alexdunnjpl I think Sean requested a change for this comment https://github.com/NASA-PDS/registry-sweepers/pull/10/files#r1192418906
No sorry Sean said it does not need change. Let's validate the PR
🗒️ Summary
⚙️ Test Data and/or Report
Ancestry sweeper needs tests as part of I&T suite (as does provenance)
Functional tests implemented/pass for ancestry sweeper Unit tests added for
pds.registrysweepers.utils.productidentifiers
♻️ Related Issues
fixes #14
supports https://github.com/NASA-PDS/registry-api/issues/318 supports https://github.com/NASA-PDS/registry-api/issues/319
Some TODOs/minor-features outstanding:
@jordanpadams if that last one isn't actually important/useful, let me know.