Improve wording and add missing documentation for rmdepcheck and license-validate - #1169
Improve wording and add missing documentation for rmdepcheck and license-validate#1169betulependule wants to merge 2 commits into
rmdepcheck and license-validate#1169Conversation
Made some improvements to make the documentation easier to read and added consistency to heading capitalization.
|
🤖 Finished Review · ✅ Success · Started 2:16 PM UTC · Completed 2:27 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.31 |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 08s |
ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| in [Testing Farm](https://testing-farm.io) run with a successful scratch build. | ||
| ### installability | ||
|
|
||
| [installability test](https://github.com/fedora-ci/installability-pipeline), |
There was a problem hiding this comment.
[low] naming-convention
The heading ### installability uses all-lowercase, diverging from the sentence-case convention used in higher-level headings (e.g., ## Scratch builds). However, within the ## Tests section, the pre-existing headings ### rpmlint and ### rpminspect already used lowercase as canonical tool names. This PR normalizes installability to match its siblings and the exact CLI identifier used in /packit-ci test installability — a defensible editorial choice.
Suggested fix: If the project prefers sentence case for all headings, change to ### Installability. Otherwise, the current lowercase is consistent with sibling test headings.
|
|
||
| ### Custom test plans | ||
| ### custom test plans | ||
|
|
There was a problem hiding this comment.
[low] naming-convention
The heading ### custom test plans uses all-lowercase, changed from the previous ### Custom test plans. Unlike tool-name headings (rpmlint, rpminspect, installability), 'custom test plans' is a descriptive phrase rather than a canonical tool identifier, so the case for keeping it capitalized is stronger. The PR is establishing a uniform lowercase style for all test sub-headings to match their CLI identifiers.
Suggested fix: Consider restoring to ### Custom test plans since this is a descriptive phrase, not a tool name. Alternatively, if the project decides all test headings should match their CLI identifiers, the lowercase is acceptable.
fullsend-ai-review
Bot
added
the
ready-for-merge
lbarcziova
left a comment
lbarcziova
left a comment
There was a problem hiding this comment.
thanks! Just few small notes
|
|
||
| ### Installability | ||
| Each of the following tests is run in [Testing Farm](https://testing-farm.io) | ||
| automatically with a successful scratch build. |
There was a problem hiding this comment.
| automatically with a successful scratch build. | |
| automatically after a successful scratch build. |
| [test](https://github.com/fedora-ci/rpminspect-pipeline) | ||
| in [Testing Farm](https://testing-farm.io) run with a successful scratch build. | ||
| [rpminspect test](https://github.com/fedora-ci/rpminspect-pipeline), which runs | ||
| [rpminspect](https://github.com/rpminspect/rpminspect) on the built RPM/SRPM packages. |
There was a problem hiding this comment.
can we add a brief description on what the tool does too, as in the rest of the test types?
| ### license-validate | ||
|
|
||
| [license-validate test](https://forge.fedoraproject.org/ci/shared-tests/src/branch/main/license-validate), | ||
| which validates that the package's license defined in the .spec file |
There was a problem hiding this comment.
| which validates that the package's license defined in the .spec file | |
| which validates that the package's license defined in the spec file |
I think we agreed on this as a norm
3 participants
Improved the wording of the Fedora CI tests sections and added missing documentation in relation to
rmdepcheckandlicense-validate.