compliantkubernetes-apps icon indicating copy to clipboard operation
compliantkubernetes-apps copied to clipboard

added documentation checklist for PR template

Open AlbinB97 opened this issue 1 year ago • 7 comments

[!warning] This is a public repository, ensure not to disclose:

  • [x] personal data beyond what is necessary for interacting with this pull request, nor
  • [x] business confidential information, such as customer names.

What kind of PR is this?

Required: Mark one of the following that is applicable:

  • [ ] kind/feature
  • [ ] kind/improvement
  • [ ] kind/deprecation
  • [ ] kind/documentation
  • [ ] kind/clean-up
  • [ ] kind/bug
  • [x] kind/other

Optional: Mark one or more of the following that are applicable:

[!important] Breaking changes should be marked kind/admin-change or kind/dev-change depending on type Critical security fixes should be marked with kind/security

  • [ ] kind/admin-change
  • [ ] kind/dev-change
  • [ ] kind/security
  • [ ] kind/adr

What does this PR do / why do we need this PR?

As discussed during meeting with @elastisys/goto-public-docs, added a checklist for documentation updates on PR template.

Information to reviewers

Checklist

  • [x] Proper commit message prefix on all commits
  • Change checks:
    • [ ] The change is transparent
    • [ ] The change is disruptive
    • [ ] The change requires no migration steps
    • [ ] The change requires migration steps
    • [ ] The change upgrades CRDs
    • [ ] The change updates the config and the schema
  • Metrics checks:
    • [ ] The metrics are still exposed and present in Grafana after the change
    • [ ] The metrics names didn't change (Grafana dashboards and Prometheus alerts are not affected)
    • [ ] The metrics names did change (Grafana dashboards and Prometheus alerts were fixed)
  • Logs checks:
    • [ ] The logs do not show any errors after the change
  • Pod Security Policy checks:
    • [ ] Any changed pod is covered by Pod Security Admission
    • [ ] Any changed pod is covered by Gatekeeper Pod Security Policies
    • [ ] The change does not cause any pods to be blocked by Pod Security Admission or Policies
  • Network Policy checks:
    • [ ] Any changed pod is covered by Network Policies
    • [ ] The change does not cause any dropped packets in the NetworkPolicy Dashboard
  • Audit checks:
    • [ ] The change does not cause any unnecessary Kubernetes audit events
    • [ ] The change requires changes to Kubernetes audit policy
  • Falco checks:
    • [ ] The change does not cause any alerts to be generated by Falco
  • Bug checks:
    • [ ] The bug fix is covered by regression tests

AlbinB97 avatar Sep 03 '24 11:09 AlbinB97

@aarnq should this be fixed in https://github.com/elastisys/ck8s-qa rather than here directly?

simonklb avatar Sep 03 '24 12:09 simonklb

@aarnq should this be fixed in https://github.com/elastisys/ck8s-qa rather than here directly?

Yes, @elastisys/goto-public-docs is this intended to be added everywhere? and why is this not integrated into the regular checklist? The template is extensive as it is and is in need of restructure to simplify the process, as the template is simply to big.

aarnq avatar Sep 03 '24 12:09 aarnq

There was a meeting around with @elastisys/goto-public-docs it to have these check boxes (See the meeting notes). It wasn't discussed if these should be in all repositories.

But personally I feel we might need to have them in all repositories and I believe @elastisys/goto-public-docs are the right people to comment on it.

salehsedghpour avatar Sep 04 '24 09:09 salehsedghpour

The reason for adding the checkbox was because we feel as if people tend to not think or check if the public docs should/need to be changed when working on tasks. So its there as a reminder to make sure we keep our docs up to date. As such it should probably be in any repo that might affect the public docs.

As far as where it should be placed. Sure it can be in the regular checklist as long as that does not "hide" it so people ignore/forget it once again. As all the items in the current checklist is not always relevant to every PR (this PR being an example). But that might be a separate discussion we need to have.

Elias-elastisys avatar Sep 04 '24 13:09 Elias-elastisys

@aarnq could you update the templates in https://github.com/elastisys/ck8s-qa with this new checkbox? Feel free to move it to the checklist instead. Also close this PR without merging when updated.

AlbinB97 avatar Sep 05 '24 11:09 AlbinB97

@aarnq could you update the templates in https://github.com/elastisys/ck8s-qa with this new checkbox? Feel free to move it to the checklist instead. Also close this PR without merging when updated.

Yes, I'll do that. I'll adapt this PR when ready.

aarnq avatar Sep 06 '24 05:09 aarnq

@elastisys/goto-public-docs PTAL

Synced some other changes from the QA repo.

aarnq avatar Sep 12 '24 13:09 aarnq

@aarnq can I merge this?

AlbinB97 avatar Sep 17 '24 08:09 AlbinB97

@aarnq can I merge this?

Yes if it looks good to you.

aarnq avatar Sep 17 '24 09:09 aarnq