Skip to content

Commit

Permalink
Fix/scrubdevel (#10475)
Browse files Browse the repository at this point in the history
Co-authored-by: Jenny Shu <[email protected]>
  • Loading branch information
nfuden and jenshu authored Jan 20, 2025
1 parent 8ec3fb1 commit e649e3b
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 173 deletions.
22 changes: 1 addition & 21 deletions ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,33 +7,13 @@

## Pull Request

### Changelog Bot
[Changelog Bot](https://github.com/solo-io/changelog-bot) ensures that changelog entries are valid.

This is enabled as a GitHub App on the project, and if changes are required, please contact the maintainers of the project by [opening an issue](/devel/contributing/issues.md).

### Github Actions
[Github Workflows](/.github/workflows) run tests which rely on Kubernetes clusters.

See the [GitHub docs](https://docs.github.com/en/actions/automating-builds-and-tests/about-continuous-integration#about-continuous-integration-using-github-actions) for more deatils about how GitHub Actions work.

### Special Labels
**Keep PR Updated**: When applied, bulldozer keeps the PR up to date with the base branch, by merging any updates into it (Applied by default).

**Work In Progress**: When applied, will prevent bulldozer from merging a PR, even if it has passed all checks.

### Special Directives to Skip CI
*If you use any of these directives, you must explain in the PR body, why this is safe*

**Skip Changelog**: Following the [special comment directives](https://github.com/solo-io/changelog-bot#issue-comment-directives), comment `/skip-changelog` on the PR. **This should rarely be used, even small changes should be documented in the changelog.**

**Skip Build-Bot**: Following the [special comment directives](https://github.com/solo-io/build-bot#issue-comment-directives), comment `/skip-ci` on the PR.

**Skip Docs Build**: Include `skipCI-docs-build:true` in the changelog entry of the PR.

**Skip Kubernetes E2E Tests**: Include `skipCI-kube-tests:true` in the changelog entry of the PR.

### Assets and Package Management
`glooctl` is built and published to the GitHub release via the script `upload_github_release_assets.go`. This is sensitive to changes to the output of `glooctl version`.

`glooctl` is also available through the package management tool [Homebrew](https://formulae.brew.sh/formula/glooctl). `glooctl` is on the [Autobump list](https://github.com/Homebrew/homebrew-core/blob/8064f66cd04d0f32dc1be25ce8363a7a9e370fae/.github/autobump.txt#L790) which means it is automatically updated within 3 hours of a stable release via [GitHub Actions within Homebrew](https://github.com/Homebrew/homebrew-core/blob/8064f66cd04d0f32dc1be25ce8363a7a9e370fae/.github/workflows/autobump.yml)
// TODO: build out directives based on updated ci
102 changes: 0 additions & 102 deletions ci/spell.sh

This file was deleted.

22 changes: 8 additions & 14 deletions devel/contributing/backports.md
Original file line number Diff line number Diff line change
@@ -1,28 +1,22 @@
# Backports

## What is a backport?
A backport is a change that is introduced on the main branch, and then applied to a previous version of Gloo Gateway.
A backport is a change that is introduced on the main branch, and then applied to a previous version of kgateway.

## When is backporting appropriate?
For a backport to be appropriate it must fit the following criteria:
- The change must have a clear rationale for why it is needed on a previous version of Gloo Gateway.
- The change must have a clear rationale for why it is needed on a previous version of kgateway.
- The change must be a low-risk change, this typically means it is a bug fix or a non-breaking change.
- The proposed change is targeted to a [supported, stable release branch](https://docs.solo.io/gloo-edge/latest/reference/support/).
- If the change is a feature request, you must have explicit approval from the product and engineering teams. This approval can also be solicited on the backport prs themselves
- Generally a backport for a feature should have been requested by at least one of these teams to be considered in the first place
- Generally a backport for a feature should have a larger discussion in the community and may need to be brought up at a community meeting.

## How to identify a backport
On the issue that tracks the desired functionality, apply a `release/1.N` label to indicate the version of Gloo Gateway you wish the request to be supported on.
On the issue that tracks the desired functionality, apply a `release/1.N` label to indicate the version of kgateway you wish the request to be supported on.

For example, if there is a `release/1.15` label, that means the issue is targeted to be introduced first on the stable main branch, and then backported to Gloo Gateway 1.15.x.

## How to create a backport
First, create a PR to introduce the change on the main branch. Doing so ensures that changes are tested and reviewed before being applied to a previous version of Gloo Gateway. Also, given that we use [protocol buffers](https://developers.google.com/protocol-buffers) for our API definitions, introducing API changes to our main branch first ensures we will not have API compatibility issues when backporting.
First, create a PR to introduce the change on the main branch. Doing so ensures that changes are tested and reviewed before being applied to a previous version of kgateway.

Once the change has been merged into the main branch, create a PR to backport the change to the desired release branch. The PR should be titled `[BRANCH NAME]: <PR title>` (ie `[1.14]: Original PR Title`). To create a backport branch we recommend the following:
- Use [cherry-pick](https://git-scm.com/docs/git-cherry-pick) to apply changes to a previous version of Gloo Gateway.
- Resolve any conflicts that have arisen due to drift between LTS branches
- If there is significant drift that causes the cherry-pick to be non-trivial, consider re-implementing the change from scratch rather than "backporting"
- Modify the changelog to be in the proper directory
- Validate that Proto fields have the same numbers as in main
- Title your pr to start with the major.minor version that you are backporting to (e.g. 1.13 for 1.13.x branch)
- Use [cherry-pick](https://git-scm.com/docs/git-cherry-pick) to apply changes to a previous version of kgateway.
- Resolve any conflicts that have arisen due to drift.
- If there is significant drift that causes the cherry-pick to be non-trivial, consider re-implementing the change from scratch rather than "backporting"
9 changes: 1 addition & 8 deletions devel/contributing/documentation.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
# Documentation

The documentation is built from a private GitHub repository. Currently, only Solo.io team members can contribute to this repository.

To request changes to the documentation or report a problem, follow the steps to [file an issue](/devel/contributing/issues.md)

## Auto-generated Docs
Please note that some of our documentation is auto-generated from source code. This includes:
- [Proto API Reference](https://docs.solo.io/gloo-edge/latest/reference/api/)
- [Helm Values](https://docs.solo.io/gloo-edge/latest/reference/helm_chart_values/)
// TODO(nfuden): link to docs pipeline setups
14 changes: 5 additions & 9 deletions devel/contributing/issues.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,17 @@
# Filing Issues

GitHub issues are the main way for reporting bugs and requesting features in Gloo Gateway.
GitHub issues are the main way for reporting bugs, tracking work you want to do and requesting features in kgateway.

If you encounter a bug or have a feature request, please take the following approach:
1. Search for existing issues.
2. If you find a similar issue, add a comment with additional information or add a 👍 reaction to indicate your agreement.
3. If there are no similar issues, [file a new one](https://github.com/solo-io/gloo/issues/new/choose)
3. If there are no similar issues, [file a new one](https://github.com/kgateway-dev/kgateway/issues/new/choose)

**Issues in this repo should not contain any sensitive information. If sensitive information is critical to the issue, please reach out to Solo.io support to open a ticket.**
**Issues in this repo should not contain any sensitive information. If sensitive information is critical to the issue, please submit via the security pipeline https://github.com/kgateway-dev/community/blob/main/CVE.md.**

## Bug Report
- Issues can include links to private resources, such as Slack conversations or private code.
- Private Enterprise issues can be used to track Open Source work.
- The more details about the issue, the better. Please include the following information:
- Gloo Gateway version
- Kgateway version
- Kubernetes version
- Operating system
- Steps to reproduce
Expand All @@ -32,7 +30,5 @@ If you encounter a bug or have a feature request, please take the following appr
- Copy the error message from the failed job (logs are discarded after a few months).

## Security Issues
We take Gloo Gateway's security very seriously. If you've found a security issue or a potential security issue in Gloo Gateway, please **DO NOT** file a public Github issue.

*Instead, send your report privately to [email protected]*
We take kgateway's security very seriously. If you've found a security issue or a potential security issue in kgateway, please **DO NOT** file a public Github issue, instead see https://github.com/kgateway-dev/community/blob/main/CVE.md for how to submit.

4 changes: 1 addition & 3 deletions devel/contributing/pull-request-reviews.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Pull Request Reviews

This doc explains the best practices for reviewing a pull request in the [Gloo Gateway project](https://github.com/solo-io/gloo).
This doc explains the best practices for reviewing a pull request in the [kgateway project](https://github.com/kgateway-dev/kgateway).
If you are looking to contribute to the project, check out the [writing pull requests guide](pull-requests.md).

- [Reviewing Pull Requests](#reviewing-pull-requests)
Expand Down Expand Up @@ -73,8 +73,6 @@ By approving a pull request, you are indicating that you have reviewed the chang
*If the changes look good to you, but you recognize that you are not the best person to review the changes, instead of approving the PR, comment that the changes look good to you and clarify why you aren't comfortable approving.*

### Open Conversations
We have branch protection rules that prevent a PR from merging when there are open conversations on a PR.

It it up to the author and reviewers to make sure all conversations are resolved, though we recommend the following strategy:
- If there is a trivial technical proposal that the PR author implemented, they can resolve the conversation when it's completed
- If there is a nit, the PR author has complete autonomy to decide how to proceed
Expand Down
22 changes: 6 additions & 16 deletions devel/contributing/pull-requests.md
Original file line number Diff line number Diff line change
@@ -1,42 +1,32 @@
# Pull Requests

This doc explains the best practices for submitting a pull request to the [Gloo Gateway project](https://github.com/solo-io/gloo).
This doc explains the best practices for submitting a pull request to the [kgateway project](https://github.com/kgateway-dev/kgateway).
It should serve as a reference for all contributors, and be useful especially useful to new and infrequent submitters.
This document serves as a [kgateway](https://github.com/kgateway-dev/kgateway) repo extension of the [org-level contribution guidelines](https://github.com/kgateway-dev/community/blob/main/CONTRIBUTING.md)



# Submission Process
Merging a pull request requires the following steps to be completed before the pull request will be merged automatically.

- [Open a pull request](https://help.github.com/articles/about-pull-requests/)
- Pass all [automated tests](automation.md)
- Get all necessary approvals from reviewers and code owners

## Marking Unfinished Pull Requests
If you want to solicit reviews before the implementation of your pull request is complete, there are a few methods to achieve this:

1. Open a Draft PR. This will run the minimum set of tests, but not the Kubernetes e2e tests
2. Remove the `Keep PR Updated` label. Automation on PRs will attempt to keep feature branches up to date with the target branch. Removing this label will prevent that and avoid CI running unnecessarily on branches that we know are not ready to merge.
3. Add the `Work In Progress` label. This will allow all tests to run will prevent the PR from merging automatically, even when all tests are passing and there are two approving reviews.

Even if you have followed these steps, it is good practice to add a `# WORK IN PROGRESS` section to the PR and outline the work that is still to be done.

## Best Practices for Pull Requests
Below are some best practices we have found to help PRs get reviewed quickly

### Follow Project Conventions
* [Coding conventions](conventions.md)

### Include a Changelog Entry
All PRs are required to contain a changelog entry. This is enforced by the [changelog bot](automation.md). If you do not include one, the first reviewer should ask for this.

You can use the [changelog.sh script](/devel/tools#changelog-creation-tool) to generate a changelog template for submission.

### Smaller Is Better
Small PRs are more likely to be reviewed quickly and thoroughly. If the PR takes **>45 minutes** to review, the review may be less thorough and more likely to miss something important.

#### Use Commits to Tell the Story
Having a series of discrete commits makes it easier to understand the idea of the PR, and break up the review into smaller chunks

When PRs merge in Gloo Gateway, they are squashed into a single commit, so it is not necessary to squash your commits before merging.
When PRs merge in kgateway, they are squashed into a single commit, so it is not necessary to squash your commits before merging.

#### Avoid Squashing Previous Commits and Using Force Pushes
This can make it difficult to understand the history of the PR, and can make it difficult to understand the changes in the future.
Expand All @@ -47,7 +37,7 @@ If you encounter cosmetic changes to the project that you wish to improve (e.g.
As always, use your judgment. A few small changes are fine, but if you find yourself making many changes to unrelated files, it is probably best to split them up.

#### Gather Feedback Early
If your changes are large, or you are unsure of the approach, it is best to gather feedback early. This can be done by opening a draft PR, or by asking for feedback in [Slack](https://slack.solo.io/).
If your changes are large, or you are unsure of the approach, it is best to gather feedback early. This can be done by opening a draft PR, or by asking for feedback in [the CNCF slack channel](https://cloud-native.slack.com/archives/C080D3PJMS4)

### Comments Matter More Over Time
In your code, if someone might not understand why you did something, they definitely won't remember later. To avoid this, add comments to your code that express the *why*, since the code should express the *how*.
Expand Down

0 comments on commit e649e3b

Please sign in to comment.