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

Move help popup generation code #65780

Merged

Conversation

GuillaumeGomez
Copy link
Member

The first commit is just a small cleanup.

The idea behind this PR is to reduce a bit more the generated HTML files by moving the duplicated code into one place instead.

r? @kinnison

@GuillaumeGomez GuillaumeGomez force-pushed the move-help-popup-generation-code branch from 1dcc915 to 7c46ed2 Compare October 24, 2019 22:37
Copy link
Contributor

@kinnison kinnison left a comment

Choose a reason for hiding this comment

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

My JavaScript-fu isn't massively strong, but afaict that looks okay. Assuming you've confirmed that it works as expected, 👍 from me.

@GuillaumeGomez
Copy link
Member Author

Let's ask someone else to review it as well then!

r? @QuietMisdreavus

@JohnCSimon JohnCSimon added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2019
@bors
Copy link
Contributor

bors commented Oct 26, 2019

☔ The latest upstream changes (presumably #65845) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez GuillaumeGomez force-pushed the move-help-popup-generation-code branch from 7c46ed2 to 91ef960 Compare October 27, 2019 10:42
@JohnCSimon
Copy link
Member

Ping from triage
@QuietMisdreavus can you please review this PR?
cc: @GuillaumeGomez
Thanks

@GuillaumeGomez
Copy link
Member Author

r? @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

I guess keyboard shortcuts don't work without JS anyway, right? So this isn't losing functionality (or documentation of that functionality)?

@GuillaumeGomez
Copy link
Member Author

Nope, just reducing generated HTML size and making page load faster for JS-disabled people (which is a complete side-effect, but a nice one I think? haha).

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 3, 2019

📌 Commit 91ef960 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 3, 2019
@bors
Copy link
Contributor

bors commented Nov 3, 2019

⌛ Testing commit 91ef960 with merge 0d5264a...

bors added a commit that referenced this pull request Nov 3, 2019
…, r=Mark-Simulacrum

Move help popup generation code

The first commit is just a small cleanup.

The idea behind this PR is to reduce a bit more the generated HTML files by moving the duplicated code into one place instead.

r? @kinnison
@bors
Copy link
Contributor

bors commented Nov 4, 2019

☀️ Test successful - checks-azure
Approved by: Mark-Simulacrum
Pushing 0d5264a to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 4, 2019
@bors bors merged commit 91ef960 into rust-lang:master Nov 4, 2019
@GuillaumeGomez GuillaumeGomez deleted the move-help-popup-generation-code branch November 4, 2019 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants