Skip to content

Latest commit

 

History

History
265 lines (214 loc) · 11.5 KB

CONTRIBUTING.md

File metadata and controls

265 lines (214 loc) · 11.5 KB
Raw

Contributing

If you're interested in joining the community to build a better SQL, there are lots of ways of contributing; big and small:

  • Star this repo.
  • Send a link to PRQL to a couple of people whose opinion you respect.
  • Subscribe to Issue #1 for updates.
  • Join the Discord.
  • Contribute towards the code. Most of the code is written in rust, and there's enough to do such that any level of experience with rust is sufficient. And if you have rust questions, there are lots of friendly people on the Discord who will patiently help you.
    • Find an issue labeled help wanted or good first issue and try to fix it. Feel free to PR partial solutions, or ask any questions on the Issue or Discord.
    • Start with something tiny! Write a test / write a docstring / make some rust nicer — it's a great way to get started by making an lasting impact in 30 minutes.
  • Contribute towards the language.
    • Find instances where the compiler produces incorrect results, and post a bug report — feel free to use the playground.
    • Open an issue / append to an existing issue with examples of queries that are difficult to express in PRQL — especially if more difficult than SQL.
    • With sufficient examples, suggest a change to the language! (Though suggestions without examples are difficult to engage with, so please do anchor suggestions in examples.)
  • Contribute towards the project.
    • Improve our website / book.
    • Tell people about PRQL.
    • Find a group of users who would be interested in PRQL, help them get up to speed, help the project understand what they need.

Any of these will inspire others to invest their time and energy into the project; thank you in advance.

Development environment

Setting up a local dev environment is simple, thanks to the rust ecosystem:

  • Install rustup & cargo.
  • That's it! Running cargo test should complete successfully.
  • For more advanced development; e.g. adjusting insta outputs or compiling for web, run task setup-dev by installing Task, or by copying & pasting them from Taskfile.yml.
  • For quick contributions, hit . in GitHub to launch a github.dev instance.
  • Any problems: post an issue and we'll help.

Commits

  • If a change is user-facing, it would be helpful to add a line in CHANGELOG.md, with {message}, ({@contributor, #X}) where X is the PR number.
  • We're experimenting with using the Conventional Commits message format, enforced through action-semantic-pull-request. This would let us generate Changelogs automatically. The check is not required to pass at the moment.

Merges

  • We merge any code that makes PRQL better
  • A PR doesn't need to be perfect to be merged; it doesn't need to solve a big problem. It needs to:
    • be in the right direction
    • make incremental progress
    • be explicit on its current state, so others can continue the progress
  • If you have merge permissions, and are reasonably confident that a PR is suitable to merge (whether or not you're the author), feel free to merge.
    • If you don't have merge permissions and have authored a few PRs, ask and ye shall receive.
  • The primary way we ratchet the code quality is through automated tests.
    • This means PRs almost always need a test to demonstrate incremental progress.
    • If a change breaks functionality without breaking tests, our tests were insufficient.
  • We use PR reviews to give general context, offer specific assistance, and collaborate on larger decisions.
    • Reviews around 'nits' like code formatting / idioms / etc are very welcome. But the norm is for them to be received as helpful advice, rather than as mandatory tasks to complete. Adding automated tests & lints to automate these suggestions is welcome.
    • If you have merge permissions and would like a PR to be reviewed before it merges, that's great — ask or assign a reviewer.
    • If a PR hasn't received attention after a day, please feel free to ping the pull request.
  • People may review a PR after it's merged. As part of the understanding that we can merge quickly, contributors are expected to incorporate this feedback into a future PR.
  • We should revert quickly if the impact of a PR turns out not to be consistent with our expectations, or there isn't as much consensus on a decision as we had hoped. It's very easy to revert code and then re-revert when we've resolved the issue; it's a sign of moving quickly.

task

We have a couple of tasks which incorporate all building & testing. While they don't need to be run as part of a standard dev loop — generally we'll want to run a more specific test — they can be useful as a backstop to ensure everything works, and as a reference for how each part of the repo is built & tested. They should be broadly consistent with the GitHub Actions workflows; please report any inconsistencies.

To build everything:

task build-all

To run all tests; (which includes building everything):

task test-all

These require installing Task, either brew install go-task/tap/go-task or as described on Task.

Tests

We use a pyramid of tests — we have fast, focused tests at the bottom of the pyramid, which give us low latency feedback when developing, and then slower, broader tests which ensure that we don't miss anything as PRQL develops1.

If you're making your first contribution, you don't need to engage with all this — it's fine to just make a change and push the results; the tests that run in GitHub will point you towards any errors, which can be then be run locally if needed. We're always around to help out.

Our tests:

  • Static checks — we run a few static checks to ensure the code stays healthy and consistent. They're defined in .pre-commit-config.yaml, using pre-commit. They can be run locally with

    pre-commit run -a

    The tests fix most of the issues they find themselves. Most of them also run on GitHub on every commit; any changes they make are added onto the branch automatically in an additional commit.

  • Unit tests & inline insta snapshots — like most projects, we rely on unit tests to test that our code basically works. We extensively use Insta, a snapshot testing tool which writes out the results of an expression in our code, making it faster to write and modify tests2.

    These are the fastest tests which run our code; they're designed to run on every save while you're developing. (While they're covered by task test-all, you'll generally want to have lower-latency tests running in a tight loop.)3

  • Integration tests — these run tests against real databases, to ensure we're producing correct SQL.

  • Examples — we compile all examples in the PRQL Book, to test that they produce the SQL we expect, and that changes to our code don't cause any unexpected regressions.

  • GitHub Actions on every commit — we run tests on prql-compiler for standard & wasm targets, and the examples in the book on every pull request every time a commit is pushed. These are designed to run in under two minutes, and we should be reassessing their scope if they grow beyond that. Once these pass, a pull request can be merged.

    All tests up to this point can be run with task test-all locally.

  • GitHub Actions on specific changes — we run additional tests on pull requests when we identify changes to some paths, such as bindings to other languages.

  • GitHub Actions on merge — we run many more tests on every merge to main. This includes testing across OSs, all our language bindings, our task tasks, a measure of test code coverage, and some performance benchmarks.

    We can run these tests before a merge by adding a label pr-test-all to the PR.

    If these tests fail after merging, we revert the merged commit before fixing the test and then re-reverting.

The goal of our tests is to allow us to make changes quickly. If you find they're making it more difficult for you to make changes, or there are missing tests that would give you the confidence to make changes faster, then please raise an issue.

Releases

Currently we release in a semi-automated way:

  • PR & merge an updated Changelog.

  • Run cargo release --no-push --no-tag -x patch locally to bump the versions, and merge the change.

  • After merging, go to Draft a new release, write up release notes, select a new tag to be created, and hit the "Publish" button.

  • From there, all packages are published automatically based on our release workflow.

  • Add in the sections for a new Changelog:

    ### Features

### Fixes

### Documentation

### Web

### Integrations

### Internal changes

We may make this more automated in future; e.g. automatic changelog creation.

Footnotes

  1. Our approach is very consistent with @matklad's advice, in his excellent blog post How to Test. ↩

  2. Here's an example of an insta test — note that only the initial line of each test is written by us; the remainder is filled in by insta. ↩

  3. For example, this is a command I frequently run:

    RUST_BACKTRACE=1 watchexec -e rs,toml,pest,md -cr -- cargo insta test --accept -- -p prql-compiler --lib

    Breaking this down:

    • RUST_BACKTRACE=1 will print a full backtrace, including where an error value was created, for rust tests which return Results.
    • watchexec -e rs,toml,pest,md -cr -- will run the subsequent command on any change to files with extensions which we are generally editing.
    • cargo insta test --accept -- runs tests with insta, a snapshot library, and writes any results immediately. I rely on git to track changes, so I run with --accept, but YMMV.
    • -p prql-compiler --lib is passed to cargo by insta; -p prql-compiler tells it to only run the tests for prql-compiler rather than the other crates, and --lib to only run the unit tests rather than the integration tests, which are much slower.
    • Note that we don't want to re-run on any file changing, because we can get into a loop of writing snapshot files, triggering a change, writing a snapshot file, etc.
    ↩