Skip to content

Use minijinja instead of handlebars #3951

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

Merged
merged 2 commits into from
Oct 11, 2021
Merged

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Sep 23, 2021

minijinja is explicitly built to be a light-weight templating system with minimal dependencies. Since we don't use a lot of the complexity that a full templating system like handlebars brings, we can reduce our number of transitive dependencies by replacing handlebars with minijinja. Note that it currently does not reduce the number of dependencies significantly, because other dependencies are still depending on these other transitive dependencies... 🙈

see https://github.com/mitsuhiko/minijinja

@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Sep 23, 2021
@bors
Copy link
Contributor

bors commented Sep 26, 2021

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

@Turbo87 Turbo87 force-pushed the minininja branch 3 times, most recently from e341c60 to 668e793 Compare October 1, 2021 15:12
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

👍 on my end but I'm not too familiar with the dump-db script. The tests should ensure it works fine but just in case, I'd like to see Justin's review. If they cannot respond for a while, feel free to r=me.

`minijinja` is explicitly built to be a light-weight templating system with minimal dependencies. Since we don't use a lot of the complexity that a full templating system like `handlebars` brings, we can reduce our number of transitive dependencies by replacing `handlebars` with `minijinja`. Note that it currently does not reduce the number of dependencies significantly, because other dependencies are still depending on these other transitive dependencies... 🙈
@Turbo87
Copy link
Member Author

Turbo87 commented Oct 11, 2021

@JohnTitor thanks! looks like Justin is currently quite busy with his new job. since we have tests for this functionality I feel quite confident that this should not break anything :)

@bors r=JohnTitor

@bors
Copy link
Contributor

bors commented Oct 11, 2021

📌 Commit bbdccd1 has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented Oct 11, 2021

⌛ Testing commit bbdccd1 with merge 3ea0c40...

@bors
Copy link
Contributor

bors commented Oct 11, 2021

☀️ Test successful - checks-actions
Approved by: JohnTitor
Pushing 3ea0c40 to master...

@bors bors merged commit 3ea0c40 into rust-lang:master Oct 11, 2021
@Turbo87 Turbo87 deleted the minininja branch October 11, 2021 17:08
@Turbo87
Copy link
Member Author

Turbo87 commented Oct 13, 2021

just imported the database dump from tonight, and everything appears to be working well :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants