Document our code review process

[leogr]

What to document

Our code review process is very similar to The Kubernetes Code Review Process (because we use the same automation of Kubernetes, i.e. Prow), but not exactly the same.

Some notable differences:

• In parallel to the Prow workflow, we enforce GitHub review too (so /approve is not enough, the maintainer also has to select the Approved checkbox and push the Submit review green button on the GitHub UI.
• For some repositories, we require a double approval (only for the falco and libs repositories), and we should explain why and when using this strategy
• A positive GitHub review implies the approved label, so one does not have to write /approve explicitly; however, that's not true for all repositories (for example, test-infra requires an explicit /approve)

Moreover, our beloved @poiana sometimes believes to be the K8s robot and writes potentially misleading messages to the users :smile_cat:

For example :point_down: https://github.com/falcosecurity/test-infra/pull/824#issuecomment-1233113339

Along with some improvements in our code review process implemented, I think we should write down a REVIEW_PROCESS.md to help contributors understand our workflow.

Thanks :grinning:

[zuc]

FTR - the way to change the link referenced in the poiana message is this one: https://github.com/kubernetes/test-infra/blob/master/prow/plugins/config.go#L323

Still, the name of the referenced page (i.e. The Kubernetes Code Review Process) can't be easily changed as it's hardcorded in the approve plugin. We'll need an upstream PR to https://github.com/kubernetes/test-infra for that.

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[poiana]

Stale issues rot after 30d of inactivity.

Mark the issue as fresh with /remove-lifecycle rotten.

Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle rotten

[jasondellaluce]

/remove-lifecycle rotten

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale /help

[poiana]

@leogr: This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed by commenting with the /remove-help command.

In response to [this](https://github.com/falcosecurity/evolution/issues/182): >/remove-lifecycle stale >/help Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

[leogr]

/remove-lifecycle stale

[h4l0gen]

Hey @leogr , if this issue is available to work on, I would like to take it on. It sounds exciting! However, I'll need some initial assistance as I haven't created a document file describing processes before, and I'm eager to learn about documentation practices. Additionally, if there are any specific preferences or requirements, please let me know so I can incorporate them effectively.

[leogr]

Hey @leogr , if this issue is available to work on, I would like to take it on. It sounds exciting! However, I'll need some initial assistance as I haven't created a document file describing processes before, and I'm eager to learn about documentation practices. Additionally, if there are any specific preferences or requirements, please let me know so I can incorporate them effectively.

Sure! You can work on this!

I recommend you become familiar with our review process first. When you feel confident, we will work on the documentation.

[h4l0gen]

Thanks, am starting with this https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process as it is nearly same process as falco have. Is this right way to start?

[leogr]

Thanks, am starting with this https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process as it is nearly same process as falco have. Is this right way to start?

Yes. I also recommend to observe @poiana (who's our @k8s-ci-robot in PRs.

[h4l0gen]

Hey @leogr, I'm done with the Falco review process and have studied the Prow documentation. If I need to reference any specific steps, I'll conduct additional research. For now, let's begin working on the documentation. Please guide me through some initial steps.

[leogr]

Hey @leogr, I'm done with the Falco review process and have studied the Prow documentation. If I need to reference any specific steps, I'll conduct additional research. For now, let's begin working on the documentation. Please guide me through some initial steps.

Would you like to start a draft?

I believe the steps would be:

• Try to add a new section, "How the Review Process Works" to https://github.com/falcosecurity/.github/blob/main/contributing/review-process.md
• Take inspiration from https://github.com/kubernetes/community/blob/master/contributors/guide/owners.md#the-code-review-process
• Adapt it according our process (see the notable differences in the description of this issue)

Then I (and I guess also @incertum) will be happy to take a look and give first feedback.

@h4l0gen Would this work for you?

[h4l0gen]

Thanks, @leogr. Yes, it works for me. I will make a draft of this shortly.

[h4l0gen]

/assign

[leogr]

/close

See https://github.com/falcosecurity/.github/blob/main/contributing/review-process.md#how-the-review-process-works

[poiana]

@leogr: Closing this issue.

In response to [this](https://github.com/falcosecurity/evolution/issues/182#issuecomment-1988934250): >/close > >See https://github.com/falcosecurity/.github/blob/main/contributing/review-process.md#how-the-review-process-works Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.