Page MenuHomeSoftware Heritage

ra: Remove global variables.
ClosedPublic

Authored by vlorentz on Nov 8 2021, 2:21 PM.

Details

Summary

Depends on D6601

There is one change in behavior: svn_special_path_non_link_data is reset at the same time as eol_style for a file. I assume it was an oversight not to do it before this diff

Diff Detail

Repository
rDLDSVN Subversion (SVN) loader
Branch
global-vars
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 24919
Build 38935: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 38934: arc lint + arc unit

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 8 2021, 2:21 PM
Harbormaster failed remote builds in B24907: Diff 24033!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 8 2021, 2:23 PM
Harbormaster failed remote builds in B24908: Diff 24034!

rebase + make the state persist

Build is green

Patch application report for D6620 (id=24045)

Rebasing onto 58a07c67d5...

Current branch diff-target is up to date.
Changes applied before test
commit befe78727ec470027c2b3595a456c01757ab3f46
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.

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

delete states recursively again

Build is green

Patch application report for D6620 (id=24046)

Rebasing onto 58a07c67d5...

Current branch diff-target is up to date.
Changes applied before test
commit 476aa30b230d0cabb8b7f341589769ee60c936d6
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.

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

ok, thanks.

Can you please add types to the constructor parameter you updated?

swh/loader/svn/ra.py
151

Please add types ;)

345

make it a more sensible debug statement instead?

This revision is now accepted and ready to land.Nov 8 2021, 4:57 PM

There is one change in behavior: svn_special_path_non_link_data is reset at the same time as eol_style for a file. I assume it was an oversight not to do it before this diff

I'm not entirely sure... But tests are ok so let's try it.

This revision now requires review to proceed.Nov 9 2021, 11:38 AM

Build is green

Patch application report for D6620 (id=24059)

Rebasing onto 189dfd5300...

Current branch diff-target is up to date.
Changes applied before test
commit 2a259e0d206862aad806a0ce48923d9f94f9aa17
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.
    
    There is one change in behavior: svn_special_path_non_link_data is reset
    at the same time as eol_style for a file.
    I assume it was an oversight not to do it before this diff

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

Looks good to me, better indeed.

I don't know why but in my mind the root Editor class was recreated between each revision load, I should have read the code of the Replay class more carefully.

swh/loader/svn/ra.py
128

Could you add a small docstring to explain the purpose of that class ?

345

not sure there is an interest to log such a debug statement here, looks like a debug print that has been accidentally committed.

This revision is now accepted and ready to land.Nov 15 2021, 1:45 PM
swh/loader/svn/ra.py
128

Any suggestion? I'm not sure I understand it myself...

345

indeed

swh/loader/svn/ra.py
128

Could be something like: Class used to persist some file states (end of lines style for instance) across revisions replay when associated subversion properties are modified.

add docstring, remove debug print

Build is green

Patch application report for D6620 (id=24134)

Rebasing onto 0f70f07bc3...

First, rewinding head to replay your work on top of it...
Applying: ra: Remove global variables.
Changes applied before test
commit d6259ee542918edcdf6abfdcabeb5bd98846dc11
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Nov 8 14:20:44 2021 +0100

    ra: Remove global variables.
    
    There is one change in behavior: svn_special_path_non_link_data is reset
    at the same time as eol_style for a file.
    I assume it was an oversight not to do it before this diff

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

This revision was landed with ongoing or failed builds.Nov 15 2021, 5:21 PM
This revision was automatically updated to reflect the committed changes.

Build is green

Patch application report for D6620 (id=24136)

Rebasing onto 65c6ae9e12...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-189-D6620.
Changes applied before test

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