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

Restore accidentally-deleted PR naming conventions #1689

Merged
merged 3 commits into from
Jul 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion docs/source/contribution/developer_contributor_guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ There are two special considerations when contributing a dataset:

## Create a pull request

Create your pull request with a descriptive title. Before you submit it, consider the following:
Create your pull request with [a descriptive title](#pull-request-title-conventions). Before you submit it, consider the following:

* You should aim for cross-platform compatibility on Windows, macOS and Linux
* We use [SemVer](https://semver.org/) for versioning
Expand All @@ -151,6 +151,17 @@ If Spark/PySpark/Hive tests for datasets are failing it might be due to the lack
We place [conftest.py](https://docs.pytest.org/en/latest/reference/fixtures.html) files in some test directories to make fixtures reusable by any tests in that directory. If you need to see which test fixtures are available and where they come from, you can issue the following command `pytest --fixtures path/to/the/test/location.py`.
```

### Pull request title conventions

The Kedro repository requires that you [squash and merge your pull request commits](https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#squash-and-merge-your-pull-request-commits), and, in most cases, the [merge message for a squash merge](https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-request-merges#merge-message-for-a-squash-merge) then defaults to the pull request title.

For clarity, your pull request title should be descriptive, and we ask you to follow some guidelines suggested by [Chris Beams](https://github.com/cbeams) in his post [How to Write a Git Commit Message](https://chris.beams.io/posts/git-commit/#seven-rules). In particular, for your pull request title, we suggest that you:

* [Limit the length to 50 characters](https://chris.beams.io/posts/git-commit/#limit-50)
* [Capitalise the first letter of the first word](https://chris.beams.io/posts/git-commit/#capitalize)
* [Omit the period at the end](https://chris.beams.io/posts/git-commit/#end)
* [Use the imperative tense](https://chris.beams.io/posts/git-commit/#imperative)

### Hints on `pre-commit` usage
[`pre-commit`](https://pre-commit.com) hooks run checks automatically on all the changed files on each commit but can be skipped with the `--no-verify` or `-n` flag:

Expand Down
8 changes: 4 additions & 4 deletions kedro/framework/project/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
from typing import Any, Dict, Optional

import click
import rich.pretty
import rich.traceback
import yaml
from dynaconf import LazySettings
from dynaconf.validator import ValidationError, Validator
from rich.pretty import install as rich_pretty_install
from rich.traceback import install as rich_traceback_install

from kedro.pipeline import Pipeline

Expand Down Expand Up @@ -200,10 +200,10 @@ def __init__(self):
# We suppress click here to hide tracebacks related to it conversely,
# kedro is not suppressed to show its tracebacks for easier debugging.
# sys.executable is used to get the kedro executable path to hide the top level traceback.
rich_traceback_install(
rich.traceback.install(
show_locals=True, suppress=[click, str(Path(sys.executable).parent)]
)
rich_pretty_install()
rich.pretty.install()
Copy link
Member Author

Choose a reason for hiding this comment

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

@AntonyMilneQB I piggy-backed a trivial change that I wanted to suggest upon reviewing some of the new rich code. I think it's better to just use the original module path, rather than from ... import ... as ... and basically end up with the same thing, with underscores replacing dots.

Copy link
Contributor

Choose a reason for hiding this comment

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

Very good suggestion, thank you!


def configure(self, logging_config: Dict[str, Any]) -> None:
"""Configure project logging using `logging_config` (e.g. from project
Expand Down