Skip to content

Commit

Permalink
Update contributor docs & PR template (#1208)
Browse files Browse the repository at this point in the history 
Update PR template and external contributor rules for PRs. Cleans up
some wording on the contributor guide as well.

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging 

- [x] Test, docs, adr added or updated as needed
- [x] [Contributor Guide
Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow)
followed
  • Loading branch information
@jeff-mccoy
jeff-mccoy authored Jan 17, 2023
1 parent ede3504 commit d358742
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 48 deletions.
19 changes: 5 additions & 14 deletions .github/pull_request_template.md
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,18 @@
## Description

<!-- Please include a summary of the change. Any relevant motivation or context is also helpful, as well as any dependencies that are required for this change -->
...

## Related Issue

<!--- This project prefers to accept pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps to reproduce -->

Fixes # (issue)
Fixes #

## Type of change

<!-- Please delete options that are not relevant -->

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Other (security config, docs update, etc)

## Checklist before merging

<!-- Please delete options that are not relevant -->

- [ ] Tests have been added/updated as necessary (add the `needs-tests` label)
- [ ] Documentation has been updated as necessary (add the `needs-docs` label)
- [ ] An ADR has been written as necessary (add the `needs-adr` label) [ [1](https://github.com/joelparkerhenderson/architecture-decision-record) [2](https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions) [3](https://adr.github.io/) ]
- [ ] Test, docs, adr added or updated as needed
- [ ] [Contributor Guide Steps](https://github.com/defenseunicorns/zarf/blob/main/CONTRIBUTING.md#developer-workflow) followed
55 changes: 21 additions & 34 deletions CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,71 +2,58 @@

First off, thanks so much for wanting to help out! :tada:

The following is a set of guidelines for contributing to Zarf. These are mostly guidelines, not rules. Use your best judgement, and feel free to propose changes to this document in a pull request.

## Developer Experience

Continuous Delivery is core to our development philosophy. Check out [https://minimumcd.org](https://minimumcd.org/) for a good baseline agreement on what that means.

Specifically:

- We do trunk-based development with short-lived feature branches that originate from the trunk, get merged to the trunk, and are deleted after the merge
- We don't merge code into the trunk that isn't releasable
- We perform automated testing on all changes before they get merged to the trunk
- We do trunk-based development (`main`) with short-lived feature branches that originate from the trunk, get merged to the trunk, and are deleted after the merge
- We don't merge code into `main` that isn't releasable
- We perform automated testing on all changes before they get merged to `main`
- We create immutable release artifacts

### Developer Workflow

Here's what a typical "day in the life" of a Zarf developer might look like. Keep in mind that other than things that are required through automation these aren't hard-and-fast rules. When in doubt, the process outlined here works for us.

:key: == Required by automation

1. Pick an outstanding issue to work on, set yourself as the assignee, and move it to "Doing Now" in the [Kanban Board](https://github.com/orgs/defenseunicorns/projects/1). The "Ready to Start" and "Planned" columns are mostly prioritized (rank order) according to feedback from our users and other inputs, but don't feel like you have to pick from the top of the pile if something else is jumping out at you.
1. Write up a rough outline of what you plan to do in the issue so you can get early feedback. Clearly announce any breaking changes you think need to be made.
1. Look at the next due [release milestone](https://github.com/defenseunicorns/zarf/milestones) and pick an issue that you want to work on. If you don't see anything that interests you, create an issue and assign it to yourself.

1. Assign yourself to the issue and drop a comment in the issue to let everyone know you're working on it.

1. :key: Set up your Git config to GPG sign all commits. [Here's some documentation on how to set it up](https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits). You won't be able to merge your PR if you have any unverified commits.

> ⚠️ **NOTE:** *If you are an external third-party contributor you will need a core-member of the zarf project to re-sign your commits; you will still receive authorship for the work you have contributed however.*
1. Create a Draft Pull Request as soon as you are able to, even if it is just 5 minutes after you started working on it. We lean towards working in the open as much as we can. If you're not sure what to put in the PR description, just put a link to the issue you're working on. If you're not sure what to put in the PR title, just put "WIP" (Work In Progress) and we'll help you out with the rest.

1. :key: Create a branch off the trunk, or a fork if you are an external contributor.
1. Create a Draft Pull Request as soon as you are able to, even if it is just 5 minutes after you started working on it. Around here we aren't afraid to show unfinished work. It helps us get feedback more quickly.
1. :key: Create a Pull Request (or mark it Ready for Review if you've been working in a Draft PR).
1. :key: Automated tests will begin based on the paths you have edited in your Pull Request. More information on these tests can be found [here](https://docs.zarf.dev/docs/developer-guide/testing).
1. Clearly announce any breaking changes that have been made.
1. :key: Get at least 1 peer-review approval.
1. :key: Merge the PR into the trunk. We tend to prefer "Squash and Merge" but if your commits are on-point and you want to preserve them in the Git history of the trunk that's fine too.
1. Delete the branch
1. Close the issue if it got fully resolved by your PR. *Hint: You can add "Fixes #XX" to the PR description to automatically close an issue when the PR is merged.*
1. :key: Automated tests will begin based on the paths you have edited in your Pull Request. More information on these tests can be found [here](https://docs.zarf.dev/docs/developer-guide/testing).

## Testing
> ⚠️ **NOTE:** _If you are an external third-party contributor, the pipelines won't run until a [CODEOWNER](https://github.com/defenseunicorns/zarf/blob/main/CODEOWNERS) approves the pipeline run._
This section dives deeper into how we test Zarf
1. :key: Be sure to use the [needs-adr,needs-docs,needs-tests](https://github.com/defenseunicorns/zarf/labels?q=needs) labels as appropriate for the PR. Once you have addressed all of the needs, remove the label.

### Pre-Commit Hooks and Linting
1. Once review is complete and approved, a core-member of the zarf project will merge your PR. If you are an external third-party contributor, two core-members of the zarf project will be required to approve the PR.

In this repo we use [pre-commit](https://pre-commit.com/) hooks for automated validation and linting. The CI pipeline will (eventually) validate that all of the hooks pass so we strongly recommend that you install the hooks locally or you'll be spending a lot of time manually fixing issues that could be fixed automatically very quickly.
1. Close the issue if it is fully resolved by your PR. _Hint: You can add "Fixes #XX" to the PR description to automatically close an issue when the PR is merged._

#### Pre-Commit Prerequisites
## Testing

This section dives deeper into how we test Zarf

1. Install [pre-commit](https://pre-commit.com/)
1. Install [go](https://golang.org/)
1. Run `pre-commit install` in the repo to install the pre-commit hooks. This will make the hooks run automatically each time you `git commit`. If you want to skip the hooks for any reason you can run `git commit --no-verify` to skip them.
### (Optional) Pre-Commit Hooks and Linting

> ℹ️ **HINT:** *Consider [automatically enabling the hooks in every Git repository](https://pre-commit.com/#automatically-enabling-pre-commit-on-repositories)*
In this repo you can optinally use [pre-commit](https://pre-commit.com/) hooks for automated validation and linting, but if not CI will run these checks for you.

### Code Testing

Our E2E tests can be found in the `/test` folder and follow the journey of someone as they would use the Zarf CLI. In CI these tests run against our currently supported cluster distros, and are the primary way that Zarf code is tested.
Our E2E tests can be found in the `/test` folder and follow the journey of someone as they would use the Zarf CLI. In CI these tests run against our currently supported cluster distros, and are the primary way that Zarf code is tested.

Our Unit tests can be found as `*_test.go` files inside the package that they are designed to test. These are also run in CI and are designed to test small functions with clear interfaces that would be difficult to test otherwise.
Our Unit tests can be found as `*_test.go` files inside the package that they are designed to test. These are also run in CI and are designed to test small functions with clear interfaces that would be difficult to test otherwise. As a general rule, we are limiting unit tests to the `src/pkg/*` folder.

All of our tests should be able to be run locally or in CI.
You can learn more about testing of Zarf [here](https://docs.zarf.dev/docs/developer-guide/testing).

> ⚠️ **NOTE:** *If you are an external third-party contributor you will need a core-member of the zarf project to run the CI tests/checks for you. It is strongly recommended to run the tests locally first as documented in the link above.*
## Documentation

In this section you'll find documentation on documentation! Pun absolutely intended :smile:

### Architecture Decision Records (ADR)

We've chosen to use ADRs to document architecturally significant decisions. We primarily use the guidance found in [this article by Michael Nygard](http://thinkrelevance.com/blog/2011/11/15/documenting-architecture-decisions) with a couple of tweaks:
Expand Down

0 comments on commit d358742

Please sign in to comment.