Skip to content

Contributing

Devin Mullins edited this page Jul 9, 2021 · 6 revisions

Cloning the repository

When cloning, use git clone -b main; this is the development branch, as opposed to the default branch releases.

A member of wg-caching will merge main into releases every ~2 months after integration testing (with the AMP validator and with Google-internal SXG validation code). If the change is urgent, it can be cherry-picked.

Running a development server

Copy amppkg.example.toml to amppkg.toml and modify it to suit your needs. You may use testdata/b1/server.{cert,privkey} to get started. However, you are at your own risk if you instruct your browser to trust server.cert. NEVER instruct your browser to trust ca.cert.

Presubmits

  • Run go fmt on the code you change. Don't run go fmt ./...; this affects files that are mirrored from Google to GitHub, so we can't change them here.
  • Make sure go test ./... passes.
  • golint and go vet are optional.
  • Make sure PRs are sent to master and not releases, unless you're releasing a new version of amppkg.

New dependencies

Feel free to add dependencies on small code if it implements a feature that's hard to implement yourself. Try not to add large dependencies, or dependencies for the sake of minor development inconvenience (unless it's for test code only). They add risk by bringing in code of unknown provenance, and bloat the binary.

If you need to add or upgrade dependencies, AMP Packager uses go modules. Please run "go mod tidy" followed by "go mod vendor" whenever you update dependencies.

What merge type?

  • For a normal PR, squash and merge.
  • For a Sync from Google, rebase and merge.
  • For a snapshot from master to releases, create a merge commit.

Working on transformers

Anything in the transformers/, internal/url/, and cmd/transform/ directory has a Google repo as its "source of truth", meaning changes have to be made there and then synced out to GitHub. We welcome PRs against this code, but reviewers should not merge the PR, but rather patch it internally. Any PRs that include changes both inside and outside these directories should be split in two (and therefore the internal half needs to be backwards-compatible).

Suggestions for contributions

Take a look at good first issues, and please communicate early and often, to ensure we agree on the solution before you invest a lot of time into its implementation.