Add style guide and code quality standards

hasheddan commented 3 years ago

What would you like to be added:

As the functionality and number of packages in krel continues to grow, we are starting to see divergence in how new features are implemented, tested, and maintained. We should add style guidelines and quality standards so that someone is completely new to the project can make contributions that are easily reviewed and integrated without degrading quality.

Why is this needed:

There are a few key areas that we have determined as important to maintain quality and consistency:

Exposing the minimum required public API in packages (i.e. bias towards private fields in structs and interfaces)
Follow pattern of "accepting interfaces, returning structs" (i.e Robustness principle)
Avoid "testing through" (i.e. make your interfaces easily mockable)
Generate mocks in a consistent manner (we use counterfeiter)
Establish and measure a (potentially flexible) threshold for unit test coverage
Establish what types of features require integration / e2e testing

This is not an all-inclusive list, but can serve as a good starting point for a document that we can continually iterate on.

/assign @hasheddan @saschagrunert

justaugustus commented 3 years ago

Thanks for opening this, Dan! /priority important-soon /milestone v1.21

puerco commented 3 years ago

+1 Thanks for pointing this out dan, I certainly would like to get some some of these well established (and understood!) to start implementing them. Sometimes I get lost and I feel waste too much time reinventing the wheel.

saschagrunert commented 3 years ago

@hasheddan when do we want to work on this issue? Seems not part of our roadmap planning but we may wire it into some side-effort.

justaugustus commented 3 years ago

Fun discussion w/ @wilsonehusin and some of the team on Twitter: https://twitter.com/stephenaugustus/status/1359354950019657730?s=20

Let's try and tackle this as a priority for v1.21.

justaugustus commented 3 years ago

/assign @puerco @wilsonehusin /unassign @hasheddan @saschagrunert /milestone v1.22

fejta-bot commented 3 years ago

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.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

fejta-bot commented 3 years ago

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.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten

k8s-triage-robot commented 3 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen
Mark this issue or PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 3 years ago

@k8s-triage-robot: Closing this issue.

In response to [this](https://github.com/kubernetes/release/issues/1694#issuecomment-903172394): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues and PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue or PR with `/reopen` >- Mark this issue or PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ 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.

justaugustus commented 2 years ago

I was thinking about this again, thanks to @helayoty! (ref: https://twitter.com/helayoty/status/1497366986841460738) /reopen

@kubernetes/release-engineering -- Thoughts here?

k8s-ci-robot commented 2 years ago

@justaugustus: Reopened this issue.

In response to [this](https://github.com/kubernetes/release/issues/1694#issuecomment-1051396725): >I was thinking about this again, thanks to @helayoty! (ref: https://twitter.com/helayoty/status/1497366986841460738) >/reopen > >@kubernetes/release-engineering -- Thoughts here? 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.

justaugustus commented 2 years ago

These are probably a good start:

justaugustus commented 2 years ago

/unassign @wilsonehusin @puerco /assign @justaugustus @helayoty

Heba, I'm going to take an initial pass using the awesome Crossplane one @hasheddan linked: https://github.com/crossplane/crossplane/blob/master/CONTRIBUTING.md

From there, we can iterate on improvements together!

helayoty commented 2 years ago

/unassign @wilsonehusin @puerco /assign @justaugustus @helayoty

Heba, I'm going to take an initial using the awesome Crossplane one @hasheddan linked: https://github.com/crossplane/crossplane/blob/master/CONTRIBUTING.md

From there, we can iterate on improvements together!

While it's so hard now to top this amazing work, I totally agree.

justaugustus commented 2 years ago

Heba, I'm going to take an initial pass using the awesome Crossplane one @hasheddan linked: https://github.com/crossplane/crossplane/blob/master/CONTRIBUTING.md

First pass: https://github.com/kubernetes/sig-release/pull/1862

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

matglas commented 2 years ago

@justaugustus the first pass form your last comment is merged.

Would a follow up for this be to reference the k/sig-release contributing doc on all sig release repos in the CONTRIBUTING.md so its clear there is sort of one central contributing guide for sig release.

Would that be basically every project in https://github.com/kubernetes/community/tree/master/sig-release#subprojects?

kubernetes / release

Add style guide and code quality standards #1694

What would you like to be added:

Why is this needed: