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

Use Mako instead of Jinja2 #1527

Merged
merged 6 commits into from
Aug 6, 2023
Merged

Use Mako instead of Jinja2 #1527

merged 6 commits into from
Aug 6, 2023

Conversation

zoli111
Copy link
Contributor

@zoli111 zoli111 commented Aug 3, 2023

Pretty descriptive title. This closes #1273.

heinezen
heinezen requested changes Aug 4, 2023
View reviewed changes
Copy link
Member

@heinezen heinezen left a comment

Choose a reason for hiding this comment

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

Code looks good and works like a charm :)

The only thing you still have to do is pass the code checks. Simply run make checkall and fix the things it complains about. Our CI does not have mako yet, so don't worry about the red Xs on GitHub.

@zoli111
Copy link
Contributor Author

zoli111 commented Aug 4, 2023

Done. I fixed all the warnings for the files I changed.

@heinezen
Copy link
Member

heinezen commented Aug 4, 2023

LGTM :)

@heinezen heinezen added improvement Enhancement of an existing component lang: python Done in Python code kevin-rebuild-pl0x instruct kevin to rebuild this pull request labels Aug 5, 2023
@SFTbot SFTbot removed the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Aug 5, 2023
@heinezen heinezen added the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Aug 5, 2023
@SFTbot SFTbot removed the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Aug 5, 2023
@TheJJ TheJJ added the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Aug 6, 2023
@SFTbot SFTbot removed the kevin-rebuild-pl0x instruct kevin to rebuild this pull request label Aug 6, 2023
TheJJ
TheJJ approved these changes Aug 6, 2023
View reviewed changes
Copy link
Member

@TheJJ TheJJ left a comment

Choose a reason for hiding this comment

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

nice! very glad to get rid of jinja, i like mako so much more :)

@TheJJ
Copy link
Member

TheJJ commented Aug 6, 2023 •
edited
Loading

Okay, the ci build now basically works, but there's some errors left, which seem unrelated to your changes and due to updates in the CI environment. It's clang only, so we can merge anyway :)

@TheJJ TheJJ merged commit 58cb4d7 into SFTtech:master Aug 6, 2023
1 check failed
@zoli111
Copy link
Contributor Author

zoli111 commented Aug 6, 2023

I'm glad I could help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheJJ TheJJ TheJJ approved these changes

@heinezen heinezen heinezen approved these changes

Assignees
No one assigned
Labels
improvement Enhancement of an existing component lang: python Done in Python code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Replace jinja2 with mako
4 participants
@zoli111 @heinezen @TheJJ @SFTbot