Skip to content

Make import name of route working for every platform#28

Merged
jb3 merged 13 commits into
mainfrom
ks123/routes-parsing
Dec 16, 2020
Merged

Make import name of route working for every platform#28
jb3 merged 13 commits into
mainfrom
ks123/routes-parsing

Conversation

@ks129
Copy link
Copy Markdown
Contributor

@ks129 ks129 commented Dec 15, 2020

As Windows use \\ instead /, current way doesn't work in Windows, because replacing / with . doesn't do anything with \\.
Now this join parent parts instead, what doesn't include separations.

Edit: Decided with @decorator-factory to merge some refactors here, more details in commits.

decorator-factory and others added 4 commits December 15, 2020 08:13
As Windows use \\ instead /, current way doesn't work in Windows, because replacing / with . doesn't do anything with \\.
Now this join parent parts instead, what doesn't include separations.
@ghost ghost added the needs 1 approval label Dec 15, 2020
@ks129 ks129 requested review from RohanJnr and jb3 December 15, 2020 06:01
Copy link
Copy Markdown
Member

@jb3 jb3 left a comment

Choose a reason for hiding this comment

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

Nice work! Few minor comments.

Comment thread backend/models/form.py
return value

def dict(self, admin: bool = True, **kwargs: t.Dict) -> t.Dict[str, t.Any]:
def dict(self, admin: bool = True, **kwargs: t.Any) -> dict[str, t.Any]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why would kwargs be t.Any? Wouldn't it make more sense to be dict[str, t.Any]?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cc @decorator-factory since I think this is a refactor.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just like *args are not hinted as tuple[str, ...], but as str

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah interesting. Thanks!

Comment thread pyproject.toml Outdated
This reverts commit eb413d6.
@ks129 ks129 requested a review from jb3 December 16, 2020 06:20
@ghost ghost removed the needs 1 approval label Dec 16, 2020
@jb3 jb3 merged commit 2c5610d into main Dec 16, 2020
@jb3 jb3 deleted the ks123/routes-parsing branch December 16, 2020 10:28
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