Skip to content

Conversation

@j-ode
Copy link
Collaborator

@j-ode j-ode commented Aug 17, 2021

Description:

  • Converted an include file containing bash functions include_mount_options_functions to a jinja macro and replaced the inclusion of said file with macro invocation.

Rationale:

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 17, 2021
@openshift-ci
Copy link

openshift-ci bot commented Aug 17, 2021

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

@j-ode j-ode requested a review from jan-cerny August 17, 2021 14:01
@jan-cerny
Copy link
Collaborator

It's an elegant solution which would allow you to proceed quickly. But, when I see it, I think it's not friendly to the developers that will want to use it, because they would have to open the macro definition and understand the bash code there. I think we should break it down and create a separate jinja macro for each of the small functions.

@openshift-ci openshift-ci bot added the needs-rebase Used by openshift-ci bot. label Aug 24, 2021
@j-ode j-ode changed the title Convert Bash functions file include_mount_options_functions to a Jinja macro Convert Bash functions from include_mount_options_functions to separate Jinja macros Aug 24, 2021
@j-ode j-ode marked this pull request as ready for review August 24, 2021 11:14
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Aug 24, 2021
@jan-cerny
Copy link
Collaborator

@j-ode Also, after you do the changes, please rebase on the latest master.

@openshift-ci openshift-ci bot removed the needs-rebase Used by openshift-ci bot. label Aug 26, 2021
@j-ode j-ode requested a review from jan-cerny August 26, 2021 14:51
@j-ode j-ode requested a review from jan-cerny August 27, 2021 10:27
@jan-cerny jan-cerny self-assigned this Aug 27, 2021
@jan-cerny jan-cerny added this to the 0.1.58 milestone Aug 27, 2021
@jan-cerny
Copy link
Collaborator

/retest

1 similar comment
@jan-cerny
Copy link
Collaborator

/retest

@openshift-ci
Copy link

openshift-ci bot commented Aug 27, 2021

@j-ode: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-ocp4-moderate-node 0bb6542 link /test e2e-aws-ocp4-moderate-node

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@jan-cerny
Copy link
Collaborator

Thank you very much for your pull request.

@jan-cerny jan-cerny merged commit 229e5d4 into ComplianceAsCode:master Aug 30, 2021
@j-ode j-ode deleted the bash_to_jinja4 branch August 30, 2021 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants