Page MenuHomeSoftware Heritage
Differential D5165

change mapping status from boolean to enum
ClosedPublic
Actions

Authored by TG1999 on Mar 1 2021, 11:09 AM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Commits
rDMFCDb8711b141781: Change Mapping Status From Boolean To Enum
Summary

Currently boolean is used for checking mapping status of a row, instead of boolean type use named class which have 3 types mapped, unmapped and ignore

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Diff Detail

Repository
rDMFCD ClearlyDefined metadata fetcher
Branch
mapping_status
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 19553
Build 30339: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 30338: arc lint + arc unit

Event Timeline

TG1999 created this revision.Mar 1 2021, 11:09 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptMar 1 2021, 11:09 AM
swh-public-ci added a comment.Mar 1 2021, 11:10 AM

Build is green

Patch application report for D5165 (id=18474)

Rebasing onto 89092343ee...

Current branch diff-target is up to date.
Changes applied before test
commit bb5cbd0b6d25a1d88b1304f33697f87f19d3ffbb
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Mon Mar 1 15:38:14 2021 +0530

    change mapping status from boolean to enum
    
    Currently boolean is used for checking mapping status of a row, instead of boolean type use named class which have 3 types mapped, unmapped and ignore
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B19551: Diff 18474.Mar 1 2021, 11:10 AM
TG1999 requested review of this revision.Mar 1 2021, 11:10 AM
TG1999 added a subscriber: vlorentz.Mar 1 2021, 11:11 AM

Hey @vlorentz please have a look !!

TG1999 updated this revision to Diff 18477.Mar 1 2021, 11:42 AM

Add case for partial mapping

swh-public-ci added a comment.Mar 1 2021, 11:43 AM

Build is green

Patch application report for D5165 (id=18477)

Rebasing onto 89092343ee...

Current branch diff-target is up to date.
Changes applied before test
commit 4a651b73c259cc92bbaaa698107a50fd080225ad
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Mon Mar 1 16:11:19 2021 +0530

    change mapping status from boolean to enum
    
    Currently boolean is used for checking mapping status of a row, instead of boolean type use named class which have 3 types mapped, unmapped and ignore
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

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

Harbormaster completed remote builds in B19553: Diff 18477.Mar 1 2021, 11:44 AM
vlorentz requested changes to this revision.Mar 1 2021, 2:27 PM

Good idea, I like it.

A few comments:

swh/clearlydefined/mapping_utils.py
263–264

when does this happen?

swh/clearlydefined/orchestrator.py
103

It does not return a MappingStatus

112–113

don't need the pass

This revision now requires changes to proceed.Mar 1 2021, 2:27 PM
TG1999 marked 2 inline comments as done.Mar 1 2021, 2:33 PM
TG1999 added inline comments.
swh/clearlydefined/mapping_utils.py
263–264

Some time revision contains tags like 0.4.0, so I don't want code to throw error on these cases, so I am checking first is it a valid hexadecimal, before mapping it with revision

vlorentz added a comment.Mar 1 2021, 2:57 PM

Good idea, I like it.

A few comments:

swh/clearlydefined/mapping_utils.py
263–264

Meh. is_hex is not going to be good enough; eg. a branch named beef would pass.

Instead, check it is hexadecimal *and* the right length (a regexp would probably be faster than the current implementation of is_hex, btw)

TG1999 added inline comments.Mar 1 2021, 5:10 PM
swh/clearlydefined/mapping_utils.py
263–264

Can you suggest me a regexp for this?

vlorentz added inline comments.Mar 1 2021, 5:17 PM
swh/clearlydefined/mapping_utils.py
263–264

I could but that's no fun. How would you do it?

TG1999 updated this revision to Diff 18517.Mar 2 2021, 10:37 AM

Add regexp

swh-public-ci added a comment.Mar 2 2021, 10:37 AM

Build has FAILED

Patch application report for D5165 (id=18517)

Rebasing onto 89092343ee...

Current branch diff-target is up to date.
Changes applied before test
commit fd948ce7cbf00b820d2a2f0556bc4c9771a34589
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Tue Mar 2 15:06:20 2021 +0530

    Change Mapping Status From Boolean To Enum
    
    Currently boolean is used for checking mapping status of a row, instead of boolean type use named class which have 3 types mapped, unmapped and ignore
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Link to build: https://jenkins.softwareheritage.org/job/DMFCD/job/tests-on-diff/119/
See console output for more information: https://jenkins.softwareheritage.org/job/DMFCD/job/tests-on-diff/119/console

Harbormaster failed remote builds in B19597: Diff 18517!Mar 2 2021, 10:38 AM
vlorentz added inline comments.Mar 2 2021, 11:27 AM
swh/clearlydefined/mapping_utils.py
63–64
vlorentz accepted this revision.Mar 2 2021, 11:27 AM
This revision is now accepted and ready to land.Mar 2 2021, 11:27 AM
TG1999 updated this revision to Diff 18530.Mar 2 2021, 3:03 PM

change function name from is_hex to is_sha1

swh-public-ci added a comment.Mar 2 2021, 3:03 PM

Build has FAILED

Patch application report for D5165 (id=18530)

Rebasing onto 89092343ee...

Current branch diff-target is up to date.
Changes applied before test
commit b8711b141781aaba5ee2d96a86c48c6989d66c07
Author: Tushar Goel <tushar.goel.dav@gmail.com>
Date:   Tue Mar 2 19:32:41 2021 +0530

    Change Mapping Status From Boolean To Enum
    
    Currently boolean is used for checking mapping status of a row, instead of boolean type use named class which have 3 types mapped, unmapped and ignore
    
    Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

Link to build: https://jenkins.softwareheritage.org/job/DMFCD/job/tests-on-diff/120/
See console output for more information: https://jenkins.softwareheritage.org/job/DMFCD/job/tests-on-diff/120/console

Harbormaster failed remote builds in B19609: Diff 18530!Mar 2 2021, 3:04 PM
This revision was landed with ongoing or failed builds.Mar 2 2021, 3:05 PM
Closed by commit rDMFCDb8711b141781: Change Mapping Status From Boolean To Enum (authored by TG1999). · Explain Why
This revision was automatically updated to reflect the committed changes.
TG1999 added a commit: rDMFCDb8711b141781: Change Mapping Status From Boolean To Enum.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
clearlydefined/
8 lines100%
Loading...
52 lines100%
Loading...
39 lines100%
Loading...
tests/
data/
2 lines--
1 line--
57 lines100%
Loading...
26 lines100%
Loading...

Diff 18477

View Options

swh/clearlydefined/error.py

Loading...
View Options

swh/clearlydefined/mapping_utils.py

Loading...
View Options

swh/clearlydefined/orchestrator.py

Loading...
View Options

swh/clearlydefined/tests/data/README.md

Loading...
View Options

swh/clearlydefined/tests/data/def_with_no_sha1_and_sha1git.json

Loading...
View Options

swh/clearlydefined/tests/test_mapping_utils.py

Loading...
View Options

swh/clearlydefined/tests/test_orchestrator.py

Loading...
About · Developer information · Legal