Skip to content

docs: include ADR to define Jinja2 boundaries #13585

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
Merged
merged 3 commits into from
Jul 21, 2025
Merged

docs: include ADR to define Jinja2 boundaries #13585

merged 3 commits into from
Jul 21, 2025

Conversation

marcusburghardt
Copy link
Member

Description:

More context and details in the ADR content.

Rationale:

  • Defining the boundaries for Jinja2 macros can help us to use the best of Jinja2 while not overusing it where it is actually not necessary.
  • This also allow external tools to more easily integrate with CaC/content in a stable and predictable way.

Review Hints:

  • The ADR is in proposed status. Feedback is welcome to polish it.
  • More PRs will be linked to the ADR highlighting the current issues we have, their contexts and their improvements.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Jun 17, 2025
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Jun 17, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@marcusburghardt
Copy link
Member Author

@Mab879 @vojtapolasek @ggbecker , it is still in draft while I am testing more cases, but could you provide an early feedback, please?

@marcusburghardt marcusburghardt mentioned this pull request Jun 18, 2025
Merged
@marcusburghardt
docs: include ADR to define jinja2 boundaries
f1093e0 
Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@marcusburghardt marcusburghardt marked this pull request as ready for review June 18, 2025 08:23
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Jun 18, 2025
@marcusburghardt marcusburghardt requested review from a team June 18, 2025 08:23
@marcusburghardt marcusburghardt changed the title docs: include ADR to define jinja2 boundaries docs: include ADR to define Jinja2 boundaries Jun 18, 2025
@marcusburghardt
Copy link
Member Author

I reviewed the initial text, included more information and context, included some PRs as reference and moved to "ready to review". I would appreciate feedback on this so we can polish the ADR and adopt better practices with Jinja2 in order to make the maintenance, integrations, and collaboration easier. Thanks. FYI @ComplianceAsCode/red-hatters @ComplianceAsCode/oracle-maintainers @ComplianceAsCode/ubuntu-maintainers @ComplianceAsCode/suse-maintainers @dahaic

@AlexXuan233 AlexXuan233 mentioned this pull request Jun 18, 2025
Merged
@marcusburghardt marcusburghardt mentioned this pull request Jun 18, 2025
Merged
AlexXuan233
AlexXuan233 reviewed Jun 18, 2025
View reviewed changes
@marcusburghardt
Include another related PR
0944608 
Thanks @RichardXuan

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@dodys dodys added this to the 0.1.78 milestone Jun 19, 2025
dodys
dodys approved these changes Jun 19, 2025
View reviewed changes
Copy link
Contributor

@dodys dodys left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

teacup-on-rockingchair
teacup-on-rockingchair approved these changes Jun 21, 2025
View reviewed changes
Copy link
Contributor

@teacup-on-rockingchair teacup-on-rockingchair left a comment

Choose a reason for hiding this comment

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

Looks great 🙇

@marcusburghardt
Update decision and status
fe2376f 
It was included an explicit paragraph in decision section to summarize
discussions around new mechanisms for the long-term. Also updated the
status after achieving the minimum required number of approvals.

Signed-off-by: Marcus Burghardt <maburgha@redhat.com>
@marcusburghardt
Copy link
Member Author

@ComplianceAsCode/red-hatters , the last commit includes a paragraph aligned to the parallel discussions we had regarding a long-term solution.

@qlty-cloud-legacy Qlty Cloud (Legacy)
Copy link

Code Climate has analyzed commit fe2376f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 61.9% (0.0% change).

View more on Code Climate.

@AlexXuan233 AlexXuan233 mentioned this pull request Jun 30, 2025
Merged
@marcusburghardt
Copy link
Member Author

@Mab879 could you help merging this, please?

@Mab879
Copy link
Member

Mab879 commented Jul 21, 2025

There are four approvals now, merging.

@Mab879 Mab879 merged commit 2c6224a into ComplianceAsCode:master Jul 21, 2025
129 of 131 checks passed
@marcusburghardt marcusburghardt deleted the adr_jinja2 branch July 23, 2025 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mab879 Mab879 Mab879 approved these changes

@teacup-on-rockingchair teacup-on-rockingchair teacup-on-rockingchair approved these changes

@dodys dodys dodys approved these changes

@Xeicker Xeicker Xeicker approved these changes

+1 more reviewer

@AlexXuan233 AlexXuan233 AlexXuan233 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.1.78

Development

Successfully merging this pull request may close these issues.

6 participants

@marcusburghardt @Mab879 @teacup-on-rockingchair @dodys @Xeicker @AlexXuan233