publish README.md

[tsipo]

Change to the pipeline to also copy the top-level README.md from master to gh-pages and commit it.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: The PR correctly adds README.md propagation to gh-pages and cleans up the chart-copy logic with proper nullglob handling. The shell scripting is generally sound, but there is one behavioral correctness issue around the NULL_SHA (initial push) path that could cause spurious README publishes, and a minor structural inconsistency with job-level outputs.

Risk level: Medium

---

Major issues:

• Spurious README publish on initial/force push (.github/workflows/helm-publish.yml, line 62): In the NULL_SHA branch, git ls-files 'README.md' always returns README.md if the file exists, so readme_changed is set to true on every initial branch push or force-push — regardless of whether the README actually changed. This is likely unintentional and will generate an unnecessary gh-pages commit. Consider explicitly documenting this intent if it is deliberate, or scope detection to an actual diff.

• readme_changed not exported as a job-level output (.github/workflows/helm-publish.yml, line 107): chart_count is a job-level output but readme_changed is only a step output. While all current consumers are within the same job, the inconsistency is a maintainability hazard as the workflow evolves.

---

Minor issues:

• shopt -s nullglob placement lacks explanatory comment (line 118): The ordering is correct but adding a comment (as is already done in the "Package" step) would aid future editors.

• Template injection vs. shell variable inconsistency (line 133): ${{ steps.package.outputs.readme_changed }} is injected directly into the shell script while packages uses a proper shell variable. The pattern is safe here (value is always true/false), but passing via an env: variable is more idiomatic and defensively avoids setting a copy-paste injection risk for future flags.

---

DRY improvement opportunities:

• The if [ "${{ steps.package.outputs.readme_changed }}" = "true" ] guard appears twice in the "Publish Charts" step (once for cp, once for git add). Refactoring into a single conditional block would reduce duplication.

---

Suggested next steps:

1. Decide intended behavior for the NULL_SHA case: if README should always be published on initial push, add a comment; if not, add a dedicated check (e.g., compare against an empty-tree SHA).
2. Add readme_changed to the job-level outputs block for completeness.
3. Consider passing readme_changed via env: in the "Publish Charts" step for consistency and safety.

[kubexautomation]

CONTENT OF THIS REVIEW IS AI GENERATED

Overall assessment: The PR cleanly adds README.md propagation to gh-pages alongside chart publishing. All four issues raised in the prior automated review have been resolved by the author — two accepted as won't-fix with good justification, two fixed in the updated diff (the env: variable for README_CHANGED and the nullglob comment). The updated code is correct and the logic is sound.

Risk level: Low

---

Minor issues (newly identified):

• Mixed comparison operators in if: condition (.github/workflows/helm-publish.yml, line 116): chart_count > 0 uses numeric coercion while readme_changed == 'true' uses string equality in the same expression. Both work correctly in GHA, but the inconsistency is a subtle readability hazard for future editors. Consider normalising to chart_count != '0' for consistency with the string-equality style already used for readme_changed.

• Unquoted/un-env'd SHA injections in shell (.github/workflows/helm-publish.yml, line 68): ${{ github.event.before }} and ${{ github.event.after }} are injected directly into the shell rather than via env: variables (pre-existing, not introduced by this PR). The values are trusted SHA hashes so there is no real risk, but this is inconsistent with the env: README_CHANGED best-practice pattern that this PR itself introduces. Worth cleaning up for consistency.

---

Suggested next steps:

1. Optionally normalise the if: condition's comparison operators to use consistent string equality.
2. Optionally move the github.event.before/github.event.after injections to env: variables with proper shell quoting (follow-up, not a blocker).
3. This PR is otherwise ready to merge.