Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: add depth parameter to LinkStorage #23

Merged
merged 3 commits into from
Apr 28, 2020

Conversation

slint
Copy link
Member

@slint slint commented Apr 17, 2020

  • Adds a "depth" parameter to the LinkStorage class which allows
    deciding the max depth at which symlinks will be created up to.

* Adds a "depth" parameter to the LinkStorage class which allows
  deciding the max depth at which symlinks will be created up to.
@slint slint force-pushed the dir-link branch 2 times, most recently from c58294d to beff591 Compare April 17, 2020 16:06
@slint slint merged commit 2947334 into inveniosoftware:master Apr 28, 2020
@slint slint deleted the dir-link branch May 15, 2020 14:52
@ThiefMaster
Copy link
Contributor

Hey, nothing wrong with dropping support for old Python versions, but silently doing so in a point release without so much as a changelog entry (and proper python_requires restrictions!) isn't cool... 😞

@slint
Copy link
Member Author

slint commented May 20, 2020

Oh God, you're right I got in "auto"-mode and I didn't even think about it :S. We've been soft-dropping Python 2 support (i.e. removing from the Travis build) in most Invenio modules in the past months.

But the intent is not to actually if possible keep Python 2 compatibility if it's easy. I can remove the os.scandir call in a v1.0.3 release. I wouldn't yet add a python_requires since I think that would then prevent anyone from installing the package with Python 2 (even if it works).

@ThiefMaster
Copy link
Contributor

ThiefMaster commented May 20, 2020

I wouldn't yet add a python_requires since I think that would then prevent anyone from installing the package with Python 2 (even if it works).

It's indeed too late for this, since you'd have to yank 1.0.2 and 1.0.3 for it to be efficient. My recommendation would be to release 1.0.4 that restores Python 2.7 support (ie no yield from). Since dropping 2.7 support in a point release could be considered a bug I think this is reasonable.

In whatever future release (2.0.0?) you explicitly drop 2.7 again, you can then add the python_requires metadata to ensure this version is not offered to unsupported versions. That way pip list --outdated also won't show it as an update if it's not supported by the current python version.

We've been soft-dropping Python 2 support (i.e. removing from the Travis build)

At least for small libraries like this one I would recommend keeping it there until you explicitly drop it (ie actualy break python 2.7 compatibility).

@lnielsen
Copy link
Member

Sorry about the mishap - I didn't realise that Indico was on Python 2.7.

We didn't consider dropping Python 2.7 tests (and thereby Python 2.7 support) as a feature change simply because of the End-Of-Life of Python 2.7, and that we implemented a policy in Invenio of not supporting unsupported Python versions. As Alex said, the intention though was not to start breaking Python 2 compatibility this fast :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants