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.

Sign up for GitHub

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

Jump to bottom

Update rinja to 0.3.2 and do some template cleanup #2591

Merged
merged 11 commits into from
Sep 4, 2024
Merged

Update rinja to 0.3.2 and do some template cleanup #2591

merged 11 commits into from
Sep 4, 2024

Conversation

GuillaumeGomez
Copy link
Member

The is (not) defined feature was improved, allowing to start some cleanups in the templates. I'm not completely done yet but I think it's big enough for now.

@GuillaumeGomez GuillaumeGomez requested a review from a team as a code owner August 21, 2024 14:59
@github-actions github-actions bot added the S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed label Aug 21, 2024
syphar
syphar requested changes Aug 22, 2024
View reviewed changes
src/web/crate_details.rs Outdated Show resolved Hide resolved
src/web/rustdoc.rs Show resolved Hide resolved
@syphar
Copy link
Member

syphar commented Aug 22, 2024

This is an awesome improvement, thanks!

@syphar syphar added S-waiting-on-author Status: This PR is incomplete or needs to address review comments and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Aug 22, 2024
@GuillaumeGomez GuillaumeGomez changed the title Update rinja to 0.3.1 and do some template cleanup Update rinja to 0.3.2 and do some template cleanup Aug 26, 2024
@GuillaumeGomez GuillaumeGomez added S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed and removed S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Aug 26, 2024
@GuillaumeGomez
Copy link
Member Author

I moved back RustdocPage to where it originally was and cleaned up the code thanks to the fix released in rinja 0.3.2.

Ready for another review round. :)

Kijewski
Kijewski suggested changes Aug 26, 2024
View reviewed changes
Copy link

@Kijewski Kijewski left a comment

Choose a reason for hiding this comment

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

I hope reviews from non-members are welcome. If not, please simply ignore me. :)

src/web/page/templates.rs Outdated Show resolved Hide resolved
templates/rustdoc/topbar.html Show resolved Hide resolved
templates/rustdoc/topbar.html Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

I hope reviews from non-members are welcome. If not, please simply ignore me. :)

It's very welcome. :)

syphar
syphar reviewed Sep 2, 2024
View reviewed changes
Copy link
Member

@syphar syphar left a comment

Choose a reason for hiding this comment

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

sorry for the delay, chaotic times.

I'm really happy about these improvements and the progress that rinja makes.

I also still feel like that CrateDetails and CrateDetailsPage should stay separate.

src/web/crate_details.rs Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

Fair enough, I'll split them again.

@GuillaumeGomez
Copy link
Member Author

Split back CrateDetails struct. :)

@GuillaumeGomez GuillaumeGomez enabled auto-merge (rebase) September 4, 2024 20:38
@GuillaumeGomez GuillaumeGomez merged commit df368ea into rust-lang:master Sep 4, 2024
10 checks passed
@GuillaumeGomez GuillaumeGomez deleted the update-rinja branch September 4, 2024 20:38
@github-actions github-actions bot added S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it and removed S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed labels Sep 4, 2024
@syphar syphar removed the S-waiting-on-deploy This PR is ready to be merged, but is waiting for an admin to have time to deploy it label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kijewski Kijewski Kijewski requested changes

@syphar syphar syphar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@GuillaumeGomez @syphar @Kijewski