Skip to content

Feature/all/base implementation#2

Open
derOtto wants to merge 20 commits intomainfrom
feature/all/base_implementation
Open

Feature/all/base implementation#2
derOtto wants to merge 20 commits intomainfrom
feature/all/base_implementation

Conversation

@derOtto
Copy link
Member

@derOtto derOtto commented Aug 2, 2021

No description provided.

@derOtto derOtto requested a review from Fra-nk August 2, 2021 00:24
Copy link
Contributor

@Fra-nk Fra-nk left a comment

Choose a reason for hiding this comment

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

LGTM 😄

Comment on lines 14 to 18
if not url or "@" in url: # empty url or url with @ symbol will always return None
return None
if "@" in url: # url with @ symbol will also return None
# TODO insert problem (why @ symbol in url? url with authentication or e-mail address?)
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

https://docs.python.org/3/library/urllib.parse.html wäre eine Idee hier.
Das Problem ist nur, dass manche gar kein "Scheme" enthalten.

ENV MODULE_NAME="bvrnapi.main"

# Install Poetry
RUN curl -sSL https://raw.githubusercontent.com/python-poetry/poetry/master/get-poetry.py | POETRY_HOME=/opt/poetry python && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, aber ich nutze bei so etwas gerne die Langform

@Fra-nk Fra-nk added the enhancement New feature or request label Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

2 participants