search

aboutcode-org / commoncode

A library of common functions shared in many other AboutCode projects
3 stars 11 forks source link

Bring back module level variables #10

Closed priv-kweihmann closed 3 years ago

priv-kweihmann commented 3 years ago

0c646b79169bcd5179902010ef4d4d7aea1cfacd removed several module level variables, although they are needed by downstream tools like scancode. Bring them back in their py3 only variants. Add test cases to avoid future regression

pombredanne commented 3 years ago

@priv-kweihmann Good catch! If you need these let me reinstate them.

Eventually they should not longer be needed by the updates to ScanCode in this branch that are about to be merged anytime: https://github.com/nexB/scancode-toolkit/pull/2365 ... so that's potentially a timing issue: previous scancode versions do not have version ranges to use (but they have requirement files with pinned wheels though)

priv-kweihmann commented 3 years ago

@priv-kweihmann Good catch! If you need these let me reinstate them.

Eventually they should not longer be needed by the updates to ScanCode in this branch that are about to be merged anytime: nexB/scancode-toolkit#2365 ... so that's potentially a timing issue: previous scancode versions do not have version ranges to use (but they have requirement files with pinned wheels though)

Alright, if something else unbreaks scancode I'm also fine with it - Let the first MR win :-)

priv-kweihmann commented 3 years ago

On a second thought, I think this still should be merged, as otherwise we will still have downstream users (current released scancode) that does pulls the current version of commoncode automatically via pip. Once scancode does actually don't require these vars anymore it's safe to revert these commits - with this you guys could control behavior via python versioning - without I see a lot of downstream jobs breaking for hard to explain reasons

pombredanne commented 3 years ago

@priv-kweihmann I wanted to merge this still. I needed to push a release first. Do you think this is no longer an issue with the latest release?

priv-kweihmann commented 3 years ago

I'm not having issues with the latest release in this regard (but with a lot of other things :-) ) - so IMO this MR can be closed as it isn't needed any more