Skip to content

Conversation

@lfrancke
Copy link
Member

No description provided.

@fhennig
Copy link
Contributor

fhennig commented Feb 22, 2023

I like it, can you also specify who should check the boxes? I.e. the reviewer or the PR author? I think sometimes I don't check them as the PR author because I think the reviewer should do die "Endabnahme" but I guess some people thing the author should check it? Not sure on this.

Re: tasklist: are you sure that it works? If I look at the markdown it looks weird: https://github.com/stackabletech/operator-templating/blob/5454d9ae311b7574d33480bda2b4da950a5919ca/template/.github/pull_request_template.md

I thought tasklist were only for issues

@lfrancke
Copy link
Member Author

Good points.
Tasklists work in PRs, yes. I tried: stackabletech/kafka-operator#557

Good point about the audience....

I could split the list in three parts but some of those things can be done by either party.
Do we want a rule for this? I'd say no but I agree that there is confusion, so it might be best to come up wtih something and just stick to that until we change it.

Reviewer

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Changelog updated (or not applicable)
  • Documentation added or updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Author

  • Changes are OpenShift compatible
  • CRD change approved (or not applicable)
  • Helm chart can be installed and deployed operator works (or not applicable)

Acceptance

  • Feature Tracker has been updated (or not applicable)
  • Proper release label has been added (or not applicable)
@fhennig
Copy link
Contributor

fhennig commented Feb 23, 2023

Yes I think someone should be made to tick the boxes, if it's not specified, boxes will remain unticked. The split looks good to me, but I'd put "Author" first because they are chronologically first I suppose

@lfrancke
Copy link
Member Author

Will do, thanks! I will update and ask for feedback once more and then we can change again as needed.

@lfrancke lfrancke requested a review from fhennig February 23, 2023 11:53
@soenkeliebau soenkeliebau merged commit cdfb266 into main Mar 8, 2023
@soenkeliebau soenkeliebau deleted the checklist branch March 8, 2023 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants