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

first draft of CONTRIBUTING.MD #73

Merged
merged 16 commits into from
Jan 24, 2022
Merged

first draft of CONTRIBUTING.MD #73

merged 16 commits into from
Jan 24, 2022

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented Nov 22, 2021

resolves #https://github.com/dbt-labs/core-team/issues/33

Description

A first draft of the possible CONTRIUBTING.MD file for the Adapter Repos

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-bigquery next" section.

@cla-bot cla-bot bot added the cla:yes label Nov 22, 2021
@McKnight-42
Copy link
Contributor Author

Would love any advice on format, if information seems clear, if we need all the same sexctions from the dbt-core Contrubting.md file etc. once we get this one done should be just a template for all the others which will make finishing assignment quick so we can take very discerning eyes with this one.

@McKnight-42 McKnight-42 self-assigned this Nov 23, 2021
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@McKnight-42 Thanks for pulling this together! Reading through this has really helped me think through what we want out of these contributing guides, and what ought to live in each adapter plugin vs. in the "core" contributing guide.

I see there's a lot of duplication with the info in the dbt-core contributing doc. I'm thinking we should:

  • Update the dbt-core contributing doc! A lot of the feedback I left below is relevant there, too
  • At the top of the dbt-bigquery contributing doc, say something like: We recommend that you first read the [core contributing doc](https://github.com/dbt-labs/dbt-core/blob/HEAD/CONTRIBUTING.md), if you haven't already. Almost all of the information there is applicable to contributing here, too! This will prevent us from needing to coordinate updates across many repos

Then, the dbt-bigquery contributing doc can include just the pieces that are different from dbt-core, which I've bolded in my comments below. The big ones are:

  • Cloning + installing dbt-bigquery locally: use pip install -e .
  • Running integration tests locally: let's detail the BIGQUERY_TEST_* env vars that are required
  • Which docs pages are likely to be impacted by user-facing changes
  • Refer to the repo/codebase/package/maintainers as dbt-bigquery throughout

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@McKnight-42 McKnight-42 requested a review from jtcohen6 November 29, 2021 20:28
Copy link
Contributor

@kwigley kwigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good, here are some initial suggestions!

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@McKnight-42 McKnight-42 requested a review from kwigley November 30, 2021 22:03
@McKnight-42 McKnight-42 marked this pull request as ready for review January 5, 2022 15:32
@McKnight-42 McKnight-42 requested a review from leahwicz January 5, 2022 15:42
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Contributor

@kwigley kwigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks good, included some tweaks

CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@McKnight-42 McKnight-42 merged commit 97acd8c into main Jan 24, 2022
@McKnight-42 McKnight-42 deleted the McKnight-42/contributing branch January 24, 2022 21:05
siephen pushed a commit to AgencyPMG/dbt-bigquery that referenced this pull request May 16, 2022
* first draft of CONTRIBUTING.MD

* second draft at adapter template

* moving section in intial setup from dbt-core to adapter repos

* added test.env.example

* making suggested changes based off kyle's review

* minor updates noticed when making first drafts for other adapter repos.

* updated test => tests for naming convention

* responding to comments on github

* continued responses to comments

* updates based on review from kyle
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants