Skip to content
This repository has been archived by the owner on Nov 29, 2022. It is now read-only.

add lang query param and document it in contributing #1034

Merged
merged 6 commits into from
Jan 25, 2018
Merged

add lang query param and document it in contributing #1034

merged 6 commits into from
Jan 25, 2018

Conversation

zeke
Copy link
Contributor

@zeke zeke commented Jan 23, 2018

This PR is a followup to #1031 that changes the one-off param from language to lang to avoid stepping on the toes of the existing default language query param name that express-request-language allows. This also adds docs to CONTRIBUTING.md so folks know they can assemble shareable translated URLs.

@zeke zeke temporarily deployed to electron-website-pr-1034 January 23, 2018 04:34 Inactive
@zeke zeke temporarily deployed to electron-website-pr-1034 January 23, 2018 04:56 Inactive
@zeke
Copy link
Contributor Author

zeke commented Jan 23, 2018

For some reason the use of supertest-session is causing the tests to hang, even though they're passing.

@zeke zeke mentioned this pull request Jan 23, 2018
Closed
@zeke zeke added enhancement New feature or request area:i18n Issues or Pull Requests related to i18n labels Jan 23, 2018
vanessayuenn
vanessayuenn approved these changes Jan 23, 2018
View reviewed changes
Copy link
Contributor

@vanessayuenn vanessayuenn left a comment

Choose a reason for hiding this comment

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

👍

While I think documenting the localized URLs is a good start, it might not be sufficient for users on the site to know that this is possible. We should consider incorporating that into the UI of the site in near future.

@zeke
Copy link
Contributor Author

zeke commented Jan 23, 2018

We should consider incorporating that into the UI

Yeah I was thinking that too. "View this page in..."

@zeke zeke had a problem deploying to electron-website-pr-1034 January 24, 2018 01:29 Failure
@zeke zeke had a problem deploying to electron-website-pr-1034 January 24, 2018 01:29 Failure
@zeke
Copy link
Contributor Author

zeke commented Jan 24, 2018

@rjs was kind enough to publish a fix to supertest-session. This should go green now.

@vanessayuenn
Copy link
Contributor

@zeke the package-lock.json was causing npm i to fail at updating supertest-session for some reason. It looks ok now, so merging this PR. :)

@vanessayuenn vanessayuenn merged commit 9dfab01 into master Jan 25, 2018
@vanessayuenn vanessayuenn deleted the language-query-fix branch January 25, 2018 13:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vanessayuenn vanessayuenn vanessayuenn approved these changes

Assignees
No one assigned
Labels
area:i18n Issues or Pull Requests related to i18n enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@zeke @vanessayuenn