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

include directory README #4685

Merged
merged 13 commits into from
Apr 6, 2022
Merged

include directory README #4685

merged 13 commits into from
Apr 6, 2022

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented Feb 4, 2022

resolves #4968

Description

Writing up a README for the include directory giving brief description of each folder or file in top level.

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

@cla-bot cla-bot bot added the cla:yes label Feb 4, 2022
core/dbt/include/README.md Outdated Show resolved Hide resolved
core/dbt/include/README.md Outdated Show resolved Hide resolved
core/dbt/include/README.md Outdated Show resolved Hide resolved
core/dbt/include/README.md Outdated Show resolved Hide resolved
@McKnight-42 McKnight-42 marked this pull request as ready for review February 15, 2022 21:30
@McKnight-42 McKnight-42 requested a review from a team as a code owner February 15, 2022 21:30
core/dbt/include/README.md Outdated Show resolved Hide resolved
@github-actions
Copy link
Contributor

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

Copy link
Member

@emmyoop emmyoop left a comment

Choose a reason for hiding this comment

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

A few tiny edits and then you'll need to generate a changelog entry with changie before merging in.

core/dbt/include/README.md Outdated Show resolved Hide resolved
core/dbt/include/README.md Outdated Show resolved Hide resolved
core/dbt/include/README.md Outdated Show resolved Hide resolved
@@ -0,0 +1,7 @@
kind: Docs
Copy link
Member

Choose a reason for hiding this comment

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

@jtcohen6 do we want Docs to just be documentation changes generally or strictly for dbt-docs changes?

Apart from this PR - it might be worth updating the changie README with more details on each change type (ie, be sure to add details of what will break on a breaking change).

Copy link
Contributor

Choose a reason for hiding this comment

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

just my opinions:

  • original intent with Docs was "strictly for dbt-docs changes"
  • generally I think a change like this can be Skip changelog — README, reflected in GitHub only, no real change to the functionality of installed dbt-core

Copy link
Member

Choose a reason for hiding this comment

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

I was going for a changelog entry for all the things! But it makes sense that this is a good candidate for Skip Changelog. @McKnight-42 can you make that happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Manually deleted draft changie file and added label.

@McKnight-42 McKnight-42 self-assigned this Apr 4, 2022
@McKnight-42 McKnight-42 added the Skip Changelog Skips GHA to check for changelog file label Apr 6, 2022
@McKnight-42 McKnight-42 requested a review from emmyoop April 6, 2022 15:50
Copy link
Member

@emmyoop emmyoop 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!

@McKnight-42 McKnight-42 merged commit 0ec829a into main Apr 6, 2022
@McKnight-42 McKnight-42 deleted the mcknight/CT-148 branch April 6, 2022 16:54
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
* start of a README for the include directory

* minor updates

* minor updates after comments from gerda and emily

* trailing space issue?

* black formatting

* minor word change

* typo update

* minor fixes and changelog creation

* remove changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes Skip Changelog Skips GHA to check for changelog file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-433] [docs] Add ReadMe to include directory
4 participants