HYPERFLEET-664 - ci: add OWNERS and lint configs for Prow CI#92
HYPERFLEET-664 - ci: add OWNERS and lint configs for Prow CI#92tzhou5 wants to merge 1 commit intoopenshift-hyperfleet:mainfrom
Conversation
WalkthroughThis pull request introduces development and build tooling infrastructure by adding configuration files for linting markdown and YAML content, an OWNERS file for code ownership, and three shell scripts to automate linting checks. The markdown linting configuration disables or relaxes numerous rules to accommodate existing documentation formatting. The YAML linting configuration similarly relaxes standard constraints. The shell scripts provide consistent entry points for running linkcheck, markdown, and YAML linting across the repository. Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
hack/yamllint.sh (1)
5-5: Use null-delimited output to handle filenames with special characters.The current
find | xargspipeline will break on filenames containing spaces, quotes, or other special characters.♻️ Proposed fix using -print0 and xargs -0
-find . -type f \( -name "*.yaml" -o -name "*.yml" \) -not -path "./.git/*" | xargs yamllint +find . -type f \( -name "*.yaml" -o -name "*.yml" \) -not -path "./.git/*" -print0 | xargs -0 yamllint🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/yamllint.sh` at line 5, The find -> xargs pipeline in hack/yamllint.sh will break on filenames with special characters; change the invocation of find used in the script (the line invoking find and xargs) to emit null-delimited results (use find ... -print0) and consume them with xargs in null-delimited mode (xargs -0) so yamllint is invoked safely for all filenames.hack/markdownlint.sh (1)
5-5: Consider excluding.gitdirectory for consistency.The
**/*.mdglob may match files in.git/(e.g.,COMMIT_EDITMSG). Whilemarkdownlint-cli2may handle this gracefully, explicitly excluding it would be consistent with the other lint scripts.♻️ Proposed fix to exclude .git
-markdownlint-cli2 "**/*.md" +markdownlint-cli2 "**/*.md" "#.git"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/markdownlint.sh` at line 5, Update the markdownlint invocation to explicitly exclude the .git directory; replace the single glob markdownlint-cli2 "**/*.md" with a command that excludes .git (for example using a negated glob pair like markdownlint-cli2 "**/*.md" "!**/.git/**" or the tool's ignore flag such as markdownlint-cli2 --ignore "**/.git/**" "**/*.md") so markdownlint-cli2 does not lint files under .git (reference the existing markdownlint-cli2 call in the script)..markdownlint-cli2.yaml (1)
1-62: Configuration disables most markdownlint rules.This configuration is extremely permissive, disabling nearly all standard markdown lint rules. While the comments document the rationale (accommodating existing docs), this effectively makes the linter a no-op for most quality checks.
Consider creating a roadmap to gradually re-enable rules as documentation is cleaned up, or at minimum keep a few high-value rules enabled (e.g.,
MD051for link fragment validation to catch broken anchor links).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.markdownlint-cli2.yaml around lines 1 - 62, The markdownlint config currently disables almost all rules (many MD### entries set to false), making linting ineffective; restore a minimal set of high-value rules (e.g., enable MD051, MD013, MD009, MD012, MD040) and add a short "roadmap" comment block in the config that lists phased re-enablement steps and owners for groups of rules (reference the MD051, MD013, MD009 symbols to locate entries), or alternatively create a separate docs/markdownlint-roadmap.md that enumerates which rules to re-enable in phases and target dates so maintainers can gradually tighten the ruleset without breaking existing docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@OWNERS`:
- Line 2: Replace the numeric GitHub ID string "86254860" with the GitHub
username "ramirez-llnl" in the OWNERS file; locate both occurrences of the
string "86254860" and change them to "ramirez-llnl" so the file uses logins
instead of numeric IDs.
---
Nitpick comments:
In @.markdownlint-cli2.yaml:
- Around line 1-62: The markdownlint config currently disables almost all rules
(many MD### entries set to false), making linting ineffective; restore a minimal
set of high-value rules (e.g., enable MD051, MD013, MD009, MD012, MD040) and add
a short "roadmap" comment block in the config that lists phased re-enablement
steps and owners for groups of rules (reference the MD051, MD013, MD009 symbols
to locate entries), or alternatively create a separate
docs/markdownlint-roadmap.md that enumerates which rules to re-enable in phases
and target dates so maintainers can gradually tighten the ruleset without
breaking existing docs.
In `@hack/markdownlint.sh`:
- Line 5: Update the markdownlint invocation to explicitly exclude the .git
directory; replace the single glob markdownlint-cli2 "**/*.md" with a command
that excludes .git (for example using a negated glob pair like markdownlint-cli2
"**/*.md" "!**/.git/**" or the tool's ignore flag such as markdownlint-cli2
--ignore "**/.git/**" "**/*.md") so markdownlint-cli2 does not lint files under
.git (reference the existing markdownlint-cli2 call in the script).
In `@hack/yamllint.sh`:
- Line 5: The find -> xargs pipeline in hack/yamllint.sh will break on filenames
with special characters; change the invocation of find used in the script (the
line invoking find and xargs) to emit null-delimited results (use find ...
-print0) and consume them with xargs in null-delimited mode (xargs -0) so
yamllint is invoked safely for all filenames.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.markdownlint-cli2.yaml.yamllintOWNERShack/linkcheck.shhack/markdownlint.shhack/yamllint.sh
| @@ -0,0 +1,35 @@ | |||
| approvers: | |||
| - "86254860" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Try to look up the GitHub user with ID 86254860
# Using both direct GitHub API and searching for this ID in the codebase
gh api /user/86254860 --jq '.login // .message // "No result"' 2>&1 || echo "GitHub CLI not available or user not found"Repository: openshift-hyperfleet/architecture
Length of output: 262
🏁 Script executed:
# Also search the codebase for any references to this ID or patterns
rg "86254860" --max-count 10Repository: openshift-hyperfleet/architecture
Length of output: 315
🏁 Script executed:
# Look at the OWNERS file structure to understand the context better
fd "OWNERS" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && cat -n {}'Repository: openshift-hyperfleet/architecture
Length of output: 247
🏁 Script executed:
# Search for other OWNERS files in the repository to see the format pattern
fd "OWNERS" -type f 2>/dev/null | xargs cat 2>/dev/null | head -50Repository: openshift-hyperfleet/architecture
Length of output: 59
🏁 Script executed:
# Look for Prow or OWNERS configuration documentation
fd "README\|OWNERS\|prow\|owners" -type f 2>/dev/null | grep -iE "(readme|doc|guide)" | head -10Repository: openshift-hyperfleet/architecture
Length of output: 59
🏁 Script executed:
# Check git history of OWNERS file to understand when/why this numeric ID was added
git log --oneline OWNERS 2>/dev/null | head -10Repository: openshift-hyperfleet/architecture
Length of output: 145
🌐 Web query:
Prow OWNERS file format GitHub usernames numeric IDs
💡 Result:
Prow’s OWNERS / OWNERS_ALIASES files are YAML and identify people by GitHub usernames (logins) (or aliases that expand to usernames) — not GitHub numeric user IDs. The key fields (reviewers, approvers, etc.) are explicitly defined as “a list of GitHub usernames or aliases,” and aliases are “a list of GitHub usernames.” [1], [2]
Example:
# OWNERS
reviewers:
- alice
approvers:
- bob# OWNERS_ALIASES (repo root)
aliases:
sig-foo:
- alice
- bob🌐 Web query:
GitHub user 86254860 username numeric ID
💡 Result:
GitHub numeric user ID 86254860 maps to a username (login) via the REST endpoint GET https://api.github.com/user/{account_id} (durable ID lookup). Use: https://api.github.com/user/86254860 and read the login field in the JSON response. [1]
cURL example
curl -L -H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
https://api.github.com/user/86254860[1]
Source
[1] https://docs.github.com/en/rest/users/users#get-a-user-using-their-id
🏁 Script executed:
# Try to fetch the GitHub user info without auth (may work for public profiles)
curl -s "https://api.github.com/user/86254860" | jq '.login' 2>/dev/null || echo "Could not fetch username"Repository: openshift-hyperfleet/architecture
Length of output: 91
🏁 Script executed:
# Check the markdown file that references this user to see if there's a hint about the username
cat hyperfleet/components/adapter/validation-deprecated/GCP/gcp-validation-adapter-spike-report.md | grep -C 3 "86254860"Repository: openshift-hyperfleet/architecture
Length of output: 648
Replace numeric GitHub ID with username ramirez-llnl.
Prow OWNERS files require GitHub usernames (logins), not numeric user IDs. Replace "86254860" with "ramirez-llnl" at both occurrences.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@OWNERS` at line 2, Replace the numeric GitHub ID string "86254860" with the
GitHub username "ramirez-llnl" in the OWNERS file; locate both occurrences of
the string "86254860" and change them to "ramirez-llnl" so the file uses logins
instead of numeric IDs.
Summary by CodeRabbit