Details

Reviewers

ardumont

Group Reviewers

Reviewers

Commits

rDMODeba8d84de660: Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID

Summary

Following the discussion on T3034, we decided to replace SWHID with
two or three classes:

QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)

This commit adds the second one

Diff Detail

Repository

rDMOD Data model

Branch

core-swhid

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 19345
Build 29997: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 29996: arc lint + arc unit

Event Timeline

vlorentz created this revision.Feb 18 2021, 1:13 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 18 2021, 1:13 PM

vlorentz planned changes to this revision.Feb 18 2021, 1:13 PM

Build is green

Patch application report for D5107 (id=18238)

Could not rebase; Attempt merge onto 0c16581283...

Updating 0c16581..a739b8b
Fast-forward
 swh/model/identifiers.py            | 223 ++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 414 ++++++++++++++++++++++++++++++++++--
 2 files changed, 614 insertions(+), 23 deletions(-)

Changes applied before test

commit a739b8b554b5184631c4912bf6e1263c3aae1f2c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit bc91ea91b3c38b96fc461097b0d55e889bcf893a
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19335: Diff 18238.Feb 18 2021, 1:14 PM

vlorentz requested review of this revision.Feb 18 2021, 1:14 PM

LGTM.

I've only added a nitpicky comment about docstrings.

swh/model/identifiers.py
995	A docstring for this one would be nice. Also, it might be worth to mention this function with an example in the class docstring, as it is the complementary feature (parsing) of pretty printing which is already documented there, and both are common needs for users of this class.

@zack added to D5081: https://forge.softwareheritage.org/D5081?vs=18237&id=18239#toc

if that's fine with you, I'll do the same here

In D5107#128987, @vlorentz wrote:

@zack added to D5081: https://forge.softwareheritage.org/D5081?vs=18237&id=18239#toc

if that's fine with you, I'll do the same here

Yeah, sure! Thanks

add serialization/deserialization example in doctest

vlorentz marked an inline comment as done.Feb 18 2021, 2:22 PM

Build is green

Patch application report for D5107 (id=18243)

Could not rebase; Attempt merge onto 0c16581283...

Updating 0c16581..7f18a88
Fast-forward
 swh/model/identifiers.py            | 238 ++++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 414 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 630 insertions(+), 23 deletions(-)

Changes applied before test

commit 7f18a887ace9a19237f8249af44c5fcd66770af3
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit c7161592c48c6853c99a453ac079170fed7ee451
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19340: Diff 18243.Feb 18 2021, 2:23 PM

swap order (QualifiedSWHID will need to reference CoreSWHID in a future diff)

Build is green

Patch application report for D5107 (id=18249)

Could not rebase; Attempt merge onto 0c16581283...

Updating 0c16581..9d6edcb
Fast-forward
 swh/model/identifiers.py            | 238 ++++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 414 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 630 insertions(+), 23 deletions(-)

Changes applied before test

commit 9d6edcb826b129571ab264e0ddf83c2b91f74807
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit c7161592c48c6853c99a453ac079170fed7ee451
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19345: Diff 18249.Feb 18 2021, 3:36 PM

rebase + remove to_dict()

Build is green

Patch application report for D5107 (id=18292)

Could not rebase; Attempt merge onto 758eb885d3...

Updating 758eb88..ad46e9a
Fast-forward
 swh/model/identifiers.py            | 232 ++++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 401 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 611 insertions(+), 23 deletions(-)

Changes applied before test

commit ad46e9a6816fa5e3d0993deabf729fa299184944
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit 50a4eaff5b29439d900ba128d93f0cb075ecc60c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19378: Diff 18292.Feb 19 2021, 10:56 AM

rebase + fix docstring + remove useless test

Build is green

Patch application report for D5107 (id=18301)

Could not rebase; Attempt merge onto 758eb885d3...

Updating 758eb88..eba8d84
Fast-forward
 swh/model/identifiers.py            | 232 +++++++++++++++++++++-
 swh/model/tests/test_identifiers.py | 377 ++++++++++++++++++++++++++++++++++--
 tox.ini                             |   1 +
 3 files changed, 587 insertions(+), 23 deletions(-)

Changes applied before test

commit eba8d84de660e2b3d7df304fa32da7404fb9f6bb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Thu Feb 18 13:09:21 2021 +0100

    Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    This commit adds the second one

commit 690b7f824f55bc65fbec21b29714926476038abb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 16 13:33:13 2021 +0100

    Add new class QualifiedSWHID to replace SWHID, and deprecate the latter.
    
    Following the discussion on T3034, we decided to replace SWHID with
    two or three classes:
    
    * QualifiedSWHID to replace the existing SWHID (standard types + qualifiers)
    * CoreSWHID, for "core SWHID" only (standard types + no qualifiers)
    * ExtendedSWHID for internal use in Software Heritage (extra types + no qualifiers)
    
    Since migrating from SWHID will break existing code, this commit uses
    the opportunity to modernize it a little, ie.:
    
    * `keyword`-only constructor, to get rid of the hacky default values for
      `object_type` and `object_id`
    * enum instead of strings for the object type
    * `bytes` instead of an hex string for the object id
    * rename `metadata` to `qualifiers`

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

Harbormaster completed remote builds in B19387: Diff 18301.Feb 19 2021, 1:53 PM

vlorentz added a child revision: D5116: QualifiedSWHID: Replace the 'qualifiers' dict with statically defined attributes.Feb 19 2021, 2:17 PM

vlorentz added a parent revision: D5081: Add new class QualifiedSWHID to replace SWHID, and deprecate the latter..

ardumont accepted this revision.Feb 22 2021, 5:51 PM

This revision is now accepted and ready to land.Feb 22 2021, 5:51 PM

Closed by commit rDMODeba8d84de660: Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID (authored by vlorentz). · Explain WhyFeb 25 2021, 3:29 PM

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDMODeba8d84de660: Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID.

Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D5107 (id=18238)

Changes applied before test

Patch application report for D5107 (id=18243)

Changes applied before test

Patch application report for D5107 (id=18249)

Changes applied before test

Patch application report for D5107 (id=18292)

Changes applied before test

Patch application report for D5107 (id=18301)

Changes applied before test

Revision Contents
Changeset List

Diff 18249

swh/model/identifiers.py

swh/model/tests/test_identifiers.py

Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHIDClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D5107 (id=18238)

Changes applied before test

Patch application report for D5107 (id=18243)

Changes applied before test

Patch application report for D5107 (id=18249)

Changes applied before test

Patch application report for D5107 (id=18292)

Changes applied before test

Patch application report for D5107 (id=18301)

Changes applied before test

Revision ContentsChangeset List

Diff 18249

swh/model/identifiers.py

swh/model/tests/test_identifiers.py

Add new class CoreSWHID as an alternative to SWHID/QualifiedSWHID
ClosedPublic
Actions

Revision Contents
Changeset List