Paths

Table of Contentst

Differential D6323

Move SWHID classes and functions from identifiers.py to swhids.py
ClosedPublic
Actions

Authored by vlorentz on Sep 22 2021, 5:47 PM.

Details

Reviewers

olasd

Group Reviewers

Reviewers

Commits

rDMOD9ec683264c41: Move SWHID classes and functions from identifiers.py to swhids.py

Summary

identifiers.py initially worked only on bare sha1_git.

I chose to add the SWHID classes in that module because
of the name, but the SWHID code didn't actually interact
with the other functions in the module, so it now feels
out of place to me.

(D6326 removes other functions)

Diff Detail

Repository

rDMOD Data model

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 23944
Build 37349: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 37348: arc lint + arc unit

Event Timeline

vlorentz created this revision.Sep 22 2021, 5:47 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptSep 22 2021, 5:47 PM

vlorentz added a child revision: D6325: Refactor identifiers & model to make *_git_object() functions work on model classes instead of dicts.Sep 22 2021, 5:47 PM

vlorentz mentioned this in D6326: Move manifest computation functions from identifiers.py to git_objects.py.Sep 22 2021, 5:49 PM

Build is green

Patch application report for D6323 (id=22977)

Rebasing onto b6f5e30b53...

Current branch diff-target is up to date.

Changes applied before test

commit d08b45c8766ec82ef9b47c30e2d574e279a0e37e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Sep 22 16:36:30 2021 +0200

    Move SWHID classes and functions from identifiers.py to swhids.py
    
    identifiers.py initially worked only on bare sha1_git.
    
    I chose to add the SWHID classes in that module because
    of the name, but the SWHID code didn't actually interact
    with the other functions in the module, so it now feels
    out of place to me.

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

Harbormaster completed remote builds in B23903: Diff 22977.Sep 22 2021, 5:49 PM

vlorentz requested review of this revision.Sep 22 2021, 5:49 PM

vlorentz edited the summary of this revision. (Show Details)Sep 22 2021, 5:49 PM

olasd added a subscriber: olasd.Sep 22 2021, 6:05 PM

olasd added inline comments.

swh/model/identifiers.py
15–16	Could we somehow wrap use of these imports in a `DeprecationWarning`? This feels like something that we'll end up keeping forever if we don't weed them out explicitly.

vlorentz added inline comments.Sep 22 2021, 6:15 PM

swh/model/identifiers.py
15–16	I'll deprecate any import of `identifiers.py` if you're fine with D6326

I'd suggest letting this one stew for a bit for others to have a chance to object :-)

swh/model/identifiers.py
15–16	Ah, even better.

This revision is now accepted and ready to land.Sep 22 2021, 6:18 PM

rebase

This revision was landed with ongoing or failed builds.Sep 23 2021, 7:55 PM

Closed by commit rDMOD9ec683264c41: Move SWHID classes and functions from identifiers.py to swhids.py (authored by vlorentz). · Explain Why

This revision was automatically updated to reflect the committed changes.

vlorentz added a commit: rDMOD9ec683264c41: Move SWHID classes and functions from identifiers.py to swhids.py.

Build is green

Patch application report for D6323 (id=23018)

Rebasing onto 0dd33cdf7d...

Current branch diff-target is up to date.

Changes applied before test

commit 9ec683264c415731286005dff823e1099ef358c3
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Sep 22 16:36:30 2021 +0200

    Move SWHID classes and functions from identifiers.py to swhids.py
    
    identifiers.py initially worked only on bare sha1_git.
    
    I chose to add the SWHID classes in that module because
    of the name, but the SWHID code didn't actually interact
    with the other functions in the module, so it now feels
    out of place to me.

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

Harbormaster completed remote builds in B23944: Diff 23018.Sep 23 2021, 7:57 PM