-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add django.org redirect URL for Individual Membership nomination form #2443
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
base: main
Are you sure you want to change the base?
Add django.org redirect URL for Individual Membership nomination form #2443
Conversation
pauloxnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good functionally, but I'd recommend adding a test, and if you have the time, moving the URL to a central place (e.g settings file)
Also, we don't use django.org domain for the website.
You can check https://github.com/django/djangoproject.com?tab=readme-ov-file#git-hooks to install git hooks locally.
|
@ulgens Thanks for the review and suggestions! |
|
Not a hard requirement but I think a test could be nice. I'm okay if others are okay with handling it later. |
|
I like the idea to add a unit test that maybe check that a redirect is in place not specifically the URL of the redirect |
|
@ulgens @pauloxnet Thanks for the clarification! |
|
@ulgens @pauloxnet I’ve added a small unit test that verifies the redirect is in place without asserting the specific target URL. Thanks for the suggestion! |
Thanks. Can you fix the linting/formatting error? |
|
@pauloxnet Thanks! I ran pre-commit locally and pushed the linting/formatting fixes. |
I think there's still a formatting issue (see the pre-commit.ci check)
I think in this case you have to manually split the URL text manually or use the short form of the same URL |
|
@pauloxnet I’ve run pre-commit locally; black reformatted the file and I’ve pushed the update. Thanks! |
pauloxnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Gave my approval, but I will recommend a squash merge. |
Fixes #2435
Adds a stable django.org URL under /foundation/ that redirects to the
Individual Membership nomination form. This avoids hard-coding a
third-party form URL and allows the underlying provider to change
without breaking links.