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

Add a section guiding through the process of rule divergence #10763

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

matejak
Copy link
Member

@matejak matejak commented Jun 28, 2023

Description:

Expand the dev guide with content regarding how to deal with changes in systems that could be typically solved either by complicating existing rules, or creating new ones. The new section makes it easier to pick the right poison.

Rationale:

Although that class of problems typically doesn't have clearly good or clearly bad solutions, approaching the problem systematically is always beneficial.

@matejak matejak added this to the 0.1.69 milestone Jun 28, 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

@marcusburghardt marcusburghardt added the Documentation Update in project documentation. label Jun 28, 2023
@Mab879 Mab879 self-assigned this Jun 28, 2023
@Mab879
Copy link
Member

Mab879 commented Jun 28, 2023

The failures on Fedora Rawhide is expected due to OpenSCAP/openscap#1995.

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR I have left many comments through the file.

I will note that I had hard time figuring out how the headings where working under "Aspects to consider when picking one approach" section. I would suggest at least using bold for text that you are using a header.

For example:

Stability

##### Conditionals

If I'm reading the markdown file I think that is something missing (it's not as bad when viewing the rendered file, but screen readers that depend section headers might get confused. I was assuming header was starting something new. However, I believe that you using Stability as some sort of header that is above (logically) Conditionals.

If you have any other questions please reach out.

docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved
docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved

By design it is expected that the rules in the project will be shared and used by the supported products. And during the lifespan of a product a component may change and require that one or more rules be updated.

When a component supported by CaC undergoes changes, it is essential to update and align the rules configuring it in the project accordingly. This is necessary to keep the rules in the project up to date and relevant.
Copy link
Member

Choose a reason for hiding this comment

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

One sentence per line.

docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved
docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved

- The ability to define extendable controls may mitigate the shortcomings mentioned right above.

Content Clarity
Copy link
Member

Choose a reason for hiding this comment

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

I would suggest making these pseudo headings bold, to help communicate these are heading like objects.

docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved
docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved
docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved
docs/manual/developer/03_creating_content.md Outdated Show resolved Hide resolved
matejak and others added 2 commits June 29, 2023 10:18
Co-authored-by: Matthew Burket <m@tthewburket.com>
and generally improve style
@codeclimate
Copy link

codeclimate bot commented Jun 29, 2023

Code Climate has analyzed commit 9a66afd 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 53.4% (0.0% change).

View more on Code Climate.

Copy link
Member

@Mab879 Mab879 left a comment

Choose a reason for hiding this comment

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

Thanks for the improvements, this is a great addition to the docs.

@Mab879
Copy link
Member

Mab879 commented Jun 29, 2023

Waving the Automatus Sanity, the fail is not related to this PR.

@Mab879 Mab879 merged commit 45a1af0 into ComplianceAsCode:master Jun 29, 2023
33 of 34 checks passed
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.

None yet

3 participants