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

Replace url with path for Django 4.0 #445

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Conversation

MuslimBeibytuly
Copy link
Contributor

No description provided.

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

why not use path()?

@MuslimBeibytuly MuslimBeibytuly changed the title Replace url with re_path for Django 4.0 Replace url with path for Django 4.0 Jan 19, 2021
@MuslimBeibytuly
Copy link
Contributor Author

why not use path()?

replaced using path, pls review:)

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

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

do we need to explicitly mention route='' , views= in the path() ?
https://docs.djangoproject.com/en/3.1/ref/urls/

though it looks nice

@MuslimBeibytuly
Copy link
Contributor Author

do we need to explicitly mention route='' , views= in the path() ?
https://docs.djangoproject.com/en/3.1/ref/urls/

though it looks nice
it isn't required, but subjectively transparent keyword args seems to be better:)

@MuslimBeibytuly
Copy link
Contributor Author

@auvipy when should I be expecting it merged and released?:)

@auvipy
Copy link
Contributor

auvipy commented Jan 19, 2021

one question, did you try this approach locally?

@nasirhjafri nasirhjafri self-requested a review January 20, 2021 12:20
@MuslimBeibytuly
Copy link
Contributor Author

one question, did you try this approach locally?

I've ran tests, if that's what you're asking about:)

@jezdez jezdez merged commit 8d9b48b into jazzband:master Jan 29, 2021
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