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

Create Vale rules for all required OCP assembly metadata #592

Open
aireilly opened this issue Jul 27, 2023 · 11 comments · May be fixed by #662
Open

Create Vale rules for all required OCP assembly metadata #592

aireilly opened this issue Jul 27, 2023 · 11 comments · May be fixed by #662
Assignees
Labels
OpenShiftAsciiDoc Issues with the OpenShiftAsciiDoc rule set only

Comments

@aireilly
Copy link
Member

aireilly commented Jul 27, 2023

Vale should check that each of the following are in place for assemblies.

  • If an attribute is found in the title, the rule should flag if the _attributes/common-attributes.adoc include is placed after the assembly title.
  • :context: value should not include _{context}
include::_attributes/common-attributes.adoc[]
:context: <unique-context-for-assembly>
= title
toc::[]
@aireilly aireilly added the OpenShiftAsciiDoc Issues with the OpenShiftAsciiDoc rule set only label Jul 27, 2023
@rohennes
Copy link
Collaborator

rohennes commented Sep 5, 2023

If an attribute is found in the title, the rule should flag if the _attributes/common-attributes.adoc include is placed after the assembly title.

We can discard this one as it's not valid anymore.
Is the second bullet point an issue you are seeing?

@aireilly
Copy link
Member Author

aireilly commented Sep 5, 2023

{_context} is used in modules to resolve unique ids. If we see :context: something_{context}, we run the risk of the previous value of :context: getting set and messing up links and URLs.

@aireilly
Copy link
Member Author

aireilly commented Sep 5, 2023

Could have sworn I saw this somewhere recently.

@aireilly
Copy link
Member Author

aireilly commented Sep 5, 2023

toc::[] should be placed after the heading.

@aireilly
Copy link
Member Author

aireilly commented Sep 5, 2023

_attributes/common-attributes.adoc should be in every assembly

@aireilly
Copy link
Member Author

aireilly commented Sep 5, 2023

🤷

@rohennes
Copy link
Collaborator

rohennes commented Sep 5, 2023

{_context} is used in modules to resolve unique ids. If we see :context: something_{context}, we run the risk of the previous value of :context: getting set and messing up links and URLs.

Sure. I searched the repo for :context:*_ but didn't get any hits. I guess no harm in having a rule to disallow it. But I haven't notice it being an issue myself.

Reverse shrug: 🤷!

@rohennes
Copy link
Collaborator

rohennes commented Sep 5, 2023

Definitely agree with the other two points. I think an assembly metadata rule could be:

  • check for the include attributes
  • check toc is under heading

And the context one if you thought it's worthwhile

@aireilly
Copy link
Member Author

aireilly commented Sep 5, 2023

Gabriel fixed a bunch of the context errors last week which is why they are no longer there.

@rohennes
Copy link
Collaborator

rohennes commented Sep 6, 2023

OK I see so we should inlude it then, a rule to check the following:

  • check for the include attributes
  • check toc is under heading
  • check the context does not include _{context}

@rohennes rohennes added the good first issue Good for newcomers label Sep 6, 2023
@rohennes rohennes removed the good first issue Good for newcomers label Sep 21, 2023
@rohennes rohennes self-assigned this Sep 21, 2023
@rohennes
Copy link
Collaborator

https://studio.vale.sh/s/691cb2f40e893adea3abf3d95071892e - something like this but perhaps should allow include to appear above/below the title

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OpenShiftAsciiDoc Issues with the OpenShiftAsciiDoc rule set only
Projects
Status: To do
Development

Successfully merging a pull request may close this issue.

2 participants