Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOCS]: CONTRIBUTING.md: as home for community contribution how-to & guidelines #289

Closed
dataders opened this issue Mar 8, 2023 · 1 comment
Labels
enhancement New feature or request

Comments

@dataders
Copy link
Contributor

dataders commented Mar 8, 2023

Describe the feature

now that integration tests have been re-enabled and automated (#204 via #274), I believe a CONTRIBUTING.md at the repo's root and referenced from README.md would be very helpful.

imo, the things that would be included are:

  1. how the integration tests run:
  2. off of any branch? or only staging? (@susodapop's comment on Update local-dev.md #284 has me confused)
  3. does a maintainer have to approve for the tests to run?
  4. similar structure to that of dbt Labs's generic contribution guidelines
    (largely similar to what's in docs/local-dev.md) covering how to:
  5. get, run, and test code locally
  6. open a PR and requirements for getting it merged (end-user CLA, update CHANGELONG.md, pass integration tests

There might also be benefit in referencing some of this content as a checklist in a Pull Request template

Who will this benefit?

All future contributors, be they Databricks or dbt Labs or from the community at large

Are you interested in contributing this feature?

Totally!

@dataders dataders added the enhancement New feature or request label Mar 8, 2023
@dataders dataders changed the title [DOCS]: CONTRIBUTING.md: as home for community contribution guidelines and [DOCS]: CONTRIBUTING.md: as home for community contribution how-to & guidelines Mar 8, 2023
@dataders dataders mentioned this issue Mar 8, 2023
6 tasks
@susodapop
Copy link

susodapop commented Mar 8, 2023

Hey there 👋 great questions. Before I merge #287 I want to make sure we address all of yours.

1 - how the integration tests are run: since these require a Databricks account and incur cloud costs we don't expect every contributor to run the whole suite repeatedly. Instructions for how to run the integration tests (which CLI commands to use) are present in the doc.

2 - Off of any branch? or only staging?: I need this to be clear, so by all means give me feedback if my wording isn't great. GitHub Actions allows us to run the workflow manually for any branch on the upstream repo but not on user forks. This is because the GH Actions workflow gets cluster connection info (including PATs) from the GH secrets API. And that API is only available to PR's of branches on the base repository. This is a security requirement to prevent hostile third parties from opening poison PR's that could leak the secrets or spam the compute resources we provision for testing.

The two-part merge process that involves a branch called staging-<PR number> is merely a convention. We could name the staging branch mickey-mouse<PR number> and it would still work. What we can't do is automatically run the integration tests from the original PR from a third-party contributor.

3 - does a maintainer have to approve for the tests to run: Absolutely yes. No integration test will run without at least three confirmation steps from a maintainer:

  1. Approve the original PR and merge feature-branch -> staging-branch
  2. Open a new PR from staging-branch -> main
  3. Manually dispatch the integration tests for the PR from step 2

4 - similar structure to that of dbt-snowflake: We'll keep these steps in separate docs for now but may integrate them in the future. I want to add a specific section about how to run a single integration test during development (which requires invoking pytest directly right now...a process that we haven't documented afaik). Consider this initial PR as the first iteration of an evolving document.

5 - get, run, and test code locally: CONTRIBUTING is not concerned with this. It's about how to get your code into the repository after your change is complete.

6 - PR requirements: The intent of CONTRIBUTING is to present this information as clearly as possible. If we're not conveying that clearly enough then it still needs work.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants