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

Create contributors guide #39

Merged
merged 5 commits into from
Jun 7, 2024
Merged

Conversation

matthieucan
Copy link
Contributor

While there, fix some inconsistencies in the documentation website, and update the readme.

Comment on lines -13 to +14
```shell
$ dbt-score lint
```
> dbt-score lint
Copy link
Contributor Author

Choose a reason for hiding this comment

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

With shell, the numbers appear in red, which is quite confusing as it's not the case in the actual terminal output

Copy link
Contributor

Choose a reason for hiding this comment

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

It is, good change 👌

Copy link
Contributor

@jochemvandooren jochemvandooren left a comment

Choose a reason for hiding this comment

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

All in all looking good! 🚀 Just a couple of comments

@@ -81,10 +81,10 @@ Every rule can be configured with the following option:
of 4.

Some rules have additional configuration options, e.g.
[sql_has_reasonable_number_of_lines](/rules/generic/#sql_has_reasonable_number_of_lines).
[sql_has_reasonable_number_of_lines](rules/generic.md#sql_has_reasonable_number_of_lines).
Copy link
Contributor

Choose a reason for hiding this comment

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

Great, I noticed these links as well 👌

README.md Outdated Show resolved Hide resolved

### Adding or changing core features

Before implementing or changing a new feature, we kindly ask you to
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we mention a lot of times that they should always open an issue first, becomes a little bit too much IMO

docs/create_rules.md Show resolved Hide resolved
Comment on lines -13 to +14
```shell
$ dbt-score lint
```
> dbt-score lint
Copy link
Contributor

Choose a reason for hiding this comment

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

It is, good change 👌

Copy link
Contributor

@druzhinin-kirill druzhinin-kirill left a comment

Choose a reason for hiding this comment

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

IMO looks good to begin with 👍

Copy link
Contributor

@jochemvandooren jochemvandooren left a comment

Choose a reason for hiding this comment

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

🚀

@matthieucan matthieucan merged commit 341eefa into master Jun 7, 2024
2 checks passed
@matthieucan matthieucan deleted the matthieucan/contributors-guide branch June 7, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants