Page MenuHomeSoftware Heritage

Make revision/release identifiers explicitly the hash of a manifest
ClosedPublic

Authored by olasd on Oct 14 2020, 6:50 PM.

Details

Summary

This collapses the shared logic between these two identifier computations into a
few more explicit steps:

  • generate data for the manifest (in either identifier computation);
  • format the manifest (in the new format_manifest function);
  • hash the manifest (in the new hash_manifest function).

This will enable reusing this logic for more object types, as well as stronger
typing for the manifest computation.

Test Plan

no changes to existing tox tests (yes, these new functions themselves should be tested)

Diff Detail

Repository
rDMOD Data model
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D4262 (id=15060)

Rebasing onto a251df2e5b...

Current branch diff-target is up to date.
Changes applied before test
commit 9224c8ca6e28cb7bc47cff104988d5ccfa2aba67
Author: Nicolas Dandrimont <nicolas@dandrimont.eu>
Date:   Wed Oct 14 18:44:53 2020 +0200

    Make revision/release identifiers explicitly the hash of a manifest
    
    This collapses the shared logic between these two identifier computations into a
    few more explicit steps:
     - generate data for the manifest (in either identifier computation);
     - format the manifest (in the new format_manifest function);
     - hash the manifest (in the new hash_manifest function).
    
    This will enable reusing this logic for more object types, as well as stronger
    typing for the manifest computation.

See https://jenkins.softwareheritage.org/job/DMOD/job/tests-on-diff/148/ for more details.

great!

It might be good to add a unit test for format_manifest, but I'm ok with the current state

This revision is now accepted and ready to land.Oct 15 2020, 10:12 AM

great!

It might be good to add a unit test for format_manifest, but I'm ok with the current state

Yeah, definitely.

After a few rounds of grep, I think there's room for more trimming down of the legacy stuff within this file before I'll tackle that