mirror of
https://github.com/neondatabase/neon.git
synced 2026-01-15 17:32:56 +00:00
75f1a01d4a
## Problem We have finite amount of runners and intermediate results are often wanted before a PR is ready for merging. Currently all PRs get e2e tests run and this creates a lot of throwaway e2e results which may or may not get to start or complete before a new push. ## Summary of changes 1. Skip e2e test when PR is in draft mode 2. Run e2e when PR status changes from draft to ready for review (change this to having its trigger in below PR and update results of build and test) 3. Abstract e2e test in a Separate workflow and call it from the main workflow for the e2e test 5. Add a label, if that label is present run e2e test in draft (run-e2e-test-in-draft) 6. Auto add a label(approve to ci) so that all the external contributors PR , e2e run in draft 7. Document the new label changes and the above behaviour Draft PR : https://github.com/neondatabase/neon/actions/runs/7729128470 Ready To Review : https://github.com/neondatabase/neon/actions/runs/7733779916 Draft PR with label : https://github.com/neondatabase/neon/actions/runs/7725691012/job/21062432342 and https://github.com/neondatabase/neon/actions/runs/7733854028 ## Checklist before requesting a review - [x] I have performed a self-review of my code. - [ ] If it is a core feature, I have added thorough tests. - [ ] Do we need to implement analytics? if so did you add the relevant metrics to the dashboard? - [ ] If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section. ## Checklist before merging - [ ] Do not forget to reformat commit message to not include the above checklist --------- Co-authored-by: Alexander Bayandin <alexander@neon.tech>
89 lines
3.9 KiB
Markdown
89 lines
3.9 KiB
Markdown
# How to contribute
|
|
|
|
Howdy! Usual good software engineering practices apply. Write
|
|
tests. Write comments. Follow standard Rust coding practices where
|
|
possible. Use `cargo fmt` and `cargo clippy` to tidy up formatting.
|
|
|
|
There are soft spots in the code, which could use cleanup,
|
|
refactoring, additional comments, and so forth. Let's try to raise the
|
|
bar, and clean things up as we go. Try to leave code in a better shape
|
|
than it was before.
|
|
|
|
## Pre-commit hook
|
|
|
|
We have a sample pre-commit hook in `pre-commit.py`.
|
|
To set it up, run:
|
|
|
|
```bash
|
|
ln -s ../../pre-commit.py .git/hooks/pre-commit
|
|
```
|
|
|
|
This will run following checks on staged files before each commit:
|
|
- `rustfmt`
|
|
- checks for Python files, see [obligatory checks](/docs/sourcetree.md#obligatory-checks).
|
|
|
|
There is also a separate script `./run_clippy.sh` that runs `cargo clippy` on the whole project
|
|
and `./scripts/reformat` that runs all formatting tools to ensure the project is up to date.
|
|
|
|
If you want to skip the hook, run `git commit` with `--no-verify` option.
|
|
|
|
## Submitting changes
|
|
|
|
1. Get at least one +1 on your PR before you push.
|
|
|
|
For simple patches, it will only take a minute for someone to review
|
|
it.
|
|
|
|
2. Don't force push small changes after making the PR ready for review.
|
|
Doing so will force readers to re-read your entire PR, which will delay
|
|
the review process.
|
|
|
|
3. Always keep the CI green.
|
|
|
|
Do not push, if the CI failed on your PR. Even if you think it's not
|
|
your patch's fault. Help to fix the root cause if something else has
|
|
broken the CI, before pushing.
|
|
|
|
*Happy Hacking!*
|
|
|
|
# How to run a CI pipeline on Pull Requests from external contributors
|
|
_An instruction for maintainers_
|
|
|
|
## TL;DR:
|
|
- Review the PR
|
|
- If and only if it looks **safe** (i.e. it doesn't contain any malicious code which could expose secrets or harm the CI), then:
|
|
- Press the "Approve and run" button in GitHub UI
|
|
- Add the `approved-for-ci-run` label to the PR
|
|
- Currently draft PR will skip e2e test (only for internal contributors). After turning the PR 'Ready to Review' CI will trigger e2e test
|
|
- Add `run-e2e-tests-in-draft` label to run e2e test in draft PR (override above behaviour)
|
|
- The `approved-for-ci-run` workflow will add `run-e2e-tests-in-draft` automatically to run e2e test for external contributors
|
|
|
|
Repeat all steps after any change to the PR.
|
|
- When the changes are ready to get merged — merge the original PR (not the internal one)
|
|
|
|
## Longer version:
|
|
|
|
GitHub Actions triggered by the `pull_request` event don't share repository secrets with the forks (for security reasons).
|
|
So, passing the CI pipeline on Pull Requests from external contributors is impossible.
|
|
|
|
We're using the following approach to make it work:
|
|
- After the review, assign the `approved-for-ci-run` label to the PR if changes look safe
|
|
- A GitHub Action will create an internal branch and a new PR with the same changes (for example, for a PR `#1234`, it'll be a branch `ci-run/pr-1234`)
|
|
- Because the PR is created from the internal branch, it is able to access repository secrets (that's why it's crucial to make sure that the PR doesn't contain any malicious code that could expose our secrets or intentionally harm the CI)
|
|
- The label gets removed automatically, so to run CI again with new changes, the label should be added again (after the review)
|
|
|
|
For details see [`approved-for-ci-run.yml`](.github/workflows/approved-for-ci-run.yml)
|
|
|
|
## How do I add the "pinned" tag to an buildtools image?
|
|
We use the `pinned` tag for `Dockerfile.buildtools` build images in our CI/CD setup, currently adding the `pinned` tag is a manual operation.
|
|
|
|
You can call it from GitHub UI: https://github.com/neondatabase/neon/actions/workflows/update_build_tools_image.yml,
|
|
or using GitHub CLI:
|
|
|
|
```bash
|
|
gh workflow -R neondatabase/neon run update_build_tools_image.yml \
|
|
-f from-tag=6254913013 \
|
|
-f to-tag=pinned \
|
|
|
|
# Default `-f to-tag` is `pinned`, so the parameter can be omitted.
|
|
``` |