Improve wording and add missing documentation for rmdepcheck and license-validate - #1169
Conversation
|
🤖 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.
There was a problem hiding this comment.
Not sure about this. If anyone prefers, I'll revert the headings changes.
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
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 1:29 PM UTC · Completed 1:29 PM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 24s |
cff3014 to
6cf7895
Compare
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 2:14 PM UTC · Completed 2:15 PM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 15s |
6cf7895 to
768778f
Compare
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 2:19 PM UTC · Completed 2:19 PM UTC Commit: Effort: high |
768778f to
cc1cb44
Compare
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 2:20 PM UTC · Completed 2:20 PM UTC Commit: Effort: high |
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 08s |
Made some improvements to make the documentation easier to read and added consistency to heading capitalization. Co-authored-by: lbarcziova <lbarcziova@users.noreply.github.com>
Co-authored-by: lbarcziova <lbarcziova@users.noreply.github.com>
cc1cb44 to
9af9908
Compare
|
Build succeeded. ✔️ pre-commit SUCCESS in 1m 44s |
|
🤖 Finished Review · ❌ Failure (creating sandbox: sandbox creation failed after 3 attempts: sandbox create failed: exit status 2 (output: error: unexpected argument '--detach' found tip: to pass '--detach' as a value, use '-- --deta…) · Started 8:22 AM UTC · Completed 8:23 AM UTC Commit: Effort: high |
|
Build succeeded (gate pipeline). ✔️ pre-commit SUCCESS in 1m 15s |
84d2089
into
packit:main
3 participants
Improved the wording of the Fedora CI tests sections and added missing documentation in relation to
rmdepcheckandlicense-validate.