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.

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

Jinja macro doc fixes #10599

Merged
merged 97 commits into from
May 23, 2023
Merged

Jinja macro doc fixes #10599

merged 97 commits into from
May 23, 2023

Conversation

maage
Copy link
Contributor

@maage maage commented May 21, 2023

Description:

Jinja macros had several issues:

  • 0,1 or 3 empty lines between macros, should be 2
  • spurious '#}}'
  • type / param format bad
  • type missing / param missing
  • type / param wrong string
  • code blocks not after ::
  • lists wrong
  • order of blocks wrong, parameters must be last
  • copy paste from somewhere and param/type had wrong names
  • some bad types

Issues not fixed here, general doc indent. Doc strings should start art beginning of the line, as any indent is visible at sphinx generated doc. Now many are intended about 4.

I did not add docs if there was none or if all :param was missing.

I think all types should conform simple python typing format. In extension of current style guide, there should be:

  • list[str] - list of strings
  • None | str - optional string

In future this could be parsed and jinja macros could be amended with checks to ensure types match.

Rationale:

This should make reference more readable, for example:

https://complianceascode.readthedocs.io/en/latest/jinja_macros/01-general.html#

Wrong doc can cause unnecessary work or bugs.

Review Hints:

I tried to limit all changes to just doc strings.

maage added 30 commits May 21, 2023 10:55
- WS at EOL
- Only one space after :param
Try to follow style guide / common examples.

Done using something like

sed -Ezi '
s/(#\}\})\n+(\{\{%)/\1\n\2/g
s/\n\n\n+/\n\n/g
s/(:param \w+:[^\n]*\n)(-?\#\}\})/\1\n\2/g
s/(:type \w+:[^\n]*\n)(-?\#\}\})/\1\n\2/g
s/(\{\{%-? endmacro -?%\}\})\n+(\{\{)/\1\n\n\n\2/g
' shared/macros/*.jinja
@openshift-ci
Copy link

openshift-ci bot commented May 21, 2023

Hi @maage. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@openshift-ci openshift-ci bot added the needs-ok-to-test Used by openshift-ci bot. label May 21, 2023
@github-actions
Copy link

Start a new ephemeral environment with changes proposed in this pull request:

Fedora Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@codeclimate
Copy link

codeclimate bot commented May 21, 2023

Code Climate has analyzed commit 1428ab1 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 52.5% (0.0% change).

View more on Code Climate.

@marcusburghardt marcusburghardt added the Documentation Update in project documentation. label May 22, 2023
@marcusburghardt marcusburghardt added this to the 0.1.68 milestone May 22, 2023
@vojtapolasek
Copy link
Collaborator

/ok-to-test

@openshift-ci openshift-ci bot added ok-to-test Used by openshift-ci bot. and removed needs-ok-to-test Used by openshift-ci bot. labels May 23, 2023
@vojtapolasek vojtapolasek self-assigned this May 23, 2023
@jan-cerny jan-cerny merged commit 255df38 into ComplianceAsCode:master May 23, 2023
@jan-cerny jan-cerny self-assigned this May 23, 2023
@jan-cerny jan-cerny removed the ok-to-test Used by openshift-ci bot. label May 23, 2023
@maage maage deleted the doc-1 branch May 23, 2023 19:05
@maage
Copy link
Contributor Author

maage commented May 23, 2023

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Update in project documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants