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

copy deprecated splitnport from stdlib Fixes #2566 #2567

Closed
wants to merge 1 commit into from

Conversation

graingert
Copy link

No description provided.

Return numerical port if a valid number are found after ':'.
Return None if ':' but not a valid number.

From https://github.com/python/cpython/blob/3.8/Lib/urllib/parse.py
Copy link
Author

@graingert graingert May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this code long enough to need a licence declaration? Imho it's fair use

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No really familiar with the Python license, but reading https://github.com/python/cpython/blob/v3.8.3/LICENSE#L74-L84

provided, however, that PSF's License Agreement and PSF's notice of copyright,
i.e., "Copyright (c) 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010,
2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Python Software Foundation;
All Rights Reserved" are retained in Python alone or in any derivative version
prepared by Licensee.

Perhaps it should be mentioned, just to be on the safe side (I guess using a range for the years to keep it a bit shorter would be ok though) 🤔

It's also better to link to a tag here instead of a release-branch (release branches could potentially be removed, which is less likely for tags). Perhaps link to the actual lines while doing so;

    Based on https://github.com/python/cpython/blob/v3.8.3/Lib/urllib/parse.py#L1094-L1108
    Copyright (c) 2001-2020 Python Software Foundation; All Rights Reserved

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah can you make this as a GitHub ```suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I avoided doing that, because overall we prefer changes to be grouped in logical commits (so no "touch-up" commits if not needed) 😅

docker/utils/utils.py Outdated Show resolved Hide resolved
Signed-off-by: Thomas Grainger <tagrain@gmail.com>
@thaJeztah
Copy link
Member

@ulyssessouza PTAL

@milas
Copy link
Contributor

milas commented Jul 26, 2022

Hi! Sorry we never got this PR over the finish line before 😓

I'm going to close this one because unfortunately there's been a lot of repo churn since this was opened, so it's no longer valid with CI, and additionally, there were actually other bugs as a result of our usage of splitnport. A fix should be forthcoming in #3004

Thanks for your contribution and time!

@milas milas closed this Jul 26, 2022
@graingert graingert deleted the patch-1 branch July 26, 2022 14:36
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.

3 participants