Skip to content

Infodev doc review #944

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Oct 26, 2022
Merged

Infodev doc review #944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
59e2136
Initial PR with doc changes
aepanchi Oct 17, 2022
d114eac
Update
aepanchi Oct 18, 2022
5621131
Merge branch 'master' into infodev-doc-review
aepanchi Oct 18, 2022
339f572
Update docs/docfiles/user_guides/QuickStart.rst
aepanchi Oct 19, 2022
dc51a38
Update README.md
aepanchi Oct 19, 2022
11cc2b6
Update with proposed suggestions
aepanchi Oct 19, 2022
64a7f67
Merge branch 'infodev-doc-review' of https://github.com/alexandraepan…
aepanchi Oct 19, 2022
3cca4f8
Final changes
aepanchi Oct 21, 2022
241b98c
Update
aepanchi Oct 25, 2022
3af9993
Ran pre-commit run --all on changes in PR
oleksandr-pavlyk Oct 26, 2022
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
6 changes: 3 additions & 3 deletions CODE_OF_CONDUCT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
## Our Pledge

In the interest of fostering an open and welcoming environment, we as
contributors and maintainers pledge to making participation in our project and
contributors and maintainers pledge to make participation in our project and
our community a harassment-free experience for everyone, regardless of age, body
size, disability, ethnicity, sex characteristics, gender identity and expression,
level of experience, education, socio-economic status, nationality, personal
appearance, race, religion, or sexual identity and orientation.

## Our Standards
# Our Standards

Examples of behavior that contributes to creating a positive environment
include:
Expand All @@ -28,7 +28,7 @@ Examples of unacceptable behavior by participants include:
* Public or private harassment
* Publishing others' private information, such as a physical or electronic
address, without explicit permission
* Other conduct which could reasonably be considered inappropriate in a
* Other conduct that could reasonably be considered inappropriate in a
professional setting

## Our Responsibilities
Expand Down
182 changes: 104 additions & 78 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,47 @@
# Contributing

We welcome your contributions. To contribute, please
- create an [issue](https://github.com/IntelPython/dpctl/issues/new),
- participate in [discussions](https://github.com/IntelPython/dpctl/discussions), or
- open a [pull request](https://github.com/IntelPython/dpctl/compare) from changes committed to your own fork
of `dpctl`.

**N.B.**: Please make sure to tick the checkmark "[x] Allow edits from maintainers" to allow proper functioning
of automation bots, see "[Working with forks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork)" GitHub document for more details.

# Contributing <!-- omit in toc -->

We welcome your contributions!

To contribute, do one of the following:
- create an [issue](https://github.com/IntelPython/dpctl/issues/new)
- participate in [discussions](https://github.com/IntelPython/dpctl/discussions)
- open a [pull request](https://github.com/IntelPython/dpctl/compare) from changes committed to your fork
of `dpctl`

> **NOTE:** Make sure to check the box ``"[x] Allow edits from maintainers"`` to allow the proper functioning
> of automation bots. See [Working with forks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/allowing-changes-to-a-pull-request-branch-created-from-a-fork) for more details.

# Table of Contents <!-- omit in toc -->
- [Mechanical Source Issues](#mechanical-source-issues)
- [Source Code Formatting](#source-code-formatting)
- [C/C++ Code Style](#cc-code-style)
- [Python Code Style](#python-code-style)
- [Setting Up a Pre-commit](#setting-up-a-pre-commit)
- [C/C++ File Headers](#cc-file-headers)
- [Python File Headers](#python-file-headers)
- [Security](#security)
- [Bandit](#bandit)
- [Code Coverage](#code-coverage)
- [Error Reporting and Logging](#error-reporting-and-logging)
- [Optional use of the Google logging library (glog)](#optional-use-of-the-google-logging-library-glog)


# Mechanical Source Issues

## Source Code Formatting

### C/C++ code style
### C/C++ Code Style

We use [clang-format](https://clang.llvm.org/docs/ClangFormat.html) code formatter.
We use [clang-format](https://clang.llvm.org/docs/ClangFormat.html) for C++ code formatting.

Install: `conda install clang-tools`
To install, run:
```bash
conda install clang-tools
```

- Revision: `10.0.1`
- See the default configuration used by dpctl in `.clang-format`.
See the default configuration used by dpctl in `.clang-format`.

Run before each commit:
Before each commit, run:

```bash
clang-format -style=file -i \
Expand All @@ -36,40 +53,38 @@ clang-format -style=file -i \
libsyclinterface/helper/source/*.cpp
```

> **_NOTE:_** A much simpler option is to use `pre-commit` and the
> `clang-format` hook that we provide.
> **NOTE:** It is recommended to use `pre-commit` that invokes `clang-format` among other linters as configured.

### Python code style
### Python Code Style

We use the following Python code style tools:
- [black](https://black.readthedocs.io/en/stable/) code formatter.
- Revision: `22.3.0`.
- [flake8](https://flake8.pycqa.org/en/latest/) linter.
- Revision `4.0.1`.
- [isort](https://pycqa.github.io/isort/) import sorter.
- Revision `5.10.1`.

- Refer `pyproject.toml` and `.flake8` config files for current configurations.
> **NOTE:** Refer:
> * `pyproject.toml` and `.flake8` config files for current configurations
> * `.pre-commit-config.yaml` file for the versions of the tools

Please run these three tools before each commit. Although, you may choose to
do so manually, but it is much easier and preferable to automate these checks.
Refer your IDE docs to set them up in your IDE, or you can set up `pre-commit`
to add git hooks.
Run these three tools before each commit.

### Setting up pre-commit
> **TIP:** Refer your IDE docs to automate these checks or set up `pre-commit` to add git hooks.

A `.pre-commit-config.yaml` is included to run various check before you
commit your code. Here are the steps to setup `pre-commit` in your workflow:
### Setting Up a Pre-commit

- Install `pre-commit`: `pip install pre-commit`
- Install the git hook scripts: `pre-commit install`
A `.pre-commit-config.yaml` is included to run various checks before you
commit your code.

To setup `pre-commit` in your workflow, install:

- `pre-commit`: `pip install pre-commit`
- the git hook scripts: `pre-commit install`

That should be it!

### C/C++ File Headers

Every C API source file should have a header on it that describes the basic
purpose of the file. The standard header looks like this:
Every C API source file should have a header that describes the file’s basic purpose.
The standard header looks like this:

```
//===----- dpctl_sycl_event_interface.h - C API for sycl::event -*-C++-*- ===//
Expand Down Expand Up @@ -97,18 +112,22 @@ purpose of the file. The standard header looks like this:
///
//===----------------------------------------------------------------------===//
```
Few things to note about this format:
- The `-*- C++ -*-` string on the first line is needed to tell Emacs that
the file is a C++ file. The string is only needed for `*.h` headers and
should be omitted for `*.cpp` files. Without the string Emacs assumes that
---
> **NOTE:**
>- The `-*- C++ -*-` string on the first line tells Emacs* that
it is a C++ file. The string is only needed for `*.h` headers and
should be omitted for `*.cpp` files. Without the string, Emacs assumes the
file is a C header.
- The copyright year should be updated every calendar year.
- Each comment line should be a max of 80 chars.
- A Doxygen `\file` tag describing the contents of the file must be provided.
Also note that the `\file` tag is inside a Doxygen comment block (
defined by `///` comment marker instead of the `//` comment marker used in the
>- The copyright year must be updated every calendar year.
>- Each comment line should be a max of 80 chars.
>- A Doxygen `\file` tag describing the contents of the file must be provided.
Note that the `\file` tag is inside a Doxygen comment block. It is
defined by the `///` comment marker instead of the `//` comment marker used in the
rest of the header.

---


### Python File Headers

Every Python and Cython file should only include the following license header:
Expand All @@ -130,42 +149,48 @@ Every Python and Cython file should only include the following license header:
# See the License for the specific language governing permissions and
# limitations under the License.
```

The copyright year should be updated every calendar year.

## Security

### Bandit

We use [Bandit](https://github.com/PyCQA/bandit) to find common security issues
in Python code.
in the Python code.

Install: `pip install bandit`
To install, run:
```bash
pip install bandit
```

- Revision: `1.7.0`
Bandit revision used: `1.7.0`

Run before each commit: `bandit -r dpctl -lll`
Before each commit, run:
```bash
bandit -r dpctl -lll
```

## Code Coverage

Code coverage for both C and Python sources in dpctl is generated for each
Code coverage, for both C and Python sources in dpctl, is generated for each
pull request (PR). A PR cannot be merged if it leads to a drop in the code
coverage by more than five percentage points. *Ergo, do not forget to write
unit tests for your changes.* To check the code coverage for your code, follow
these steps:
coverage by more than five percentage points. Therefore, write
unit tests for your changes.

To check the code coverage for your code, follow these steps:

1. Install dependencies for C/C++ source.

For C/C++ source we require `lcov`, `llvm-cov`, and `llvm-profdata`. Note
that `llvm-cov` and `llvm-profdata` should be version 11.0 or higher. If you
have multiple `llvm-cov` tools installed, most likely because you have
multiple llvm installations, you should set the `LLVM_TOOLS_HOME`
For C/C++ source, `lcov`, `llvm-cov` (>=11.0), and `llvm-profdata` (>=11.0) are required. If you
have multiple `llvm-cov` tools installed, set the `LLVM_TOOLS_HOME`
environment variable to make sure the correct one is used to generate
coverage.

2. Install dependencies for Python sources.

To generate the coverage data for dpctl's Python sources, you only need to
install `coverage`.
To generate the coverage data for dpctl Python sources,
install `coverage`:

```bash
python -m pip install coverage[toml]
Expand All @@ -178,13 +203,13 @@ these steps:
coverage html
```

Note that code coverage builds the C sources with debug symbols. For this
The code coverage builds the C sources with debug symbols. For this
reason, the coverage script builds the package in `develop` mode of
`setup.py`.

The coverage results for the C and Python sources will be printed to the
terminal during the build (C API) and during the pytest execution (Python).
The detailed coverage reports for the C API is saved to the
The coverage results for the C and Python sources are printed to the
terminal during the build (`libsyclinterface`) and pytest execution (Python).
The detailed coverage reports for the `libsyclinterface` library are saved to the
`dpctl-c-api-coverage` directory. The Python coverage reports are saved to
the `htmlcov` directory.

Expand All @@ -201,37 +226,39 @@ these steps:
> ^
> 1 error generated.
> ```
> The error is related to the `tcl` package. You should uninstall the `tcl`
> package to resolve the error.
> The error is related to the `tcl` package. Uninstall the `tcl`
> package to resolve it.

## Error Reporting and Logging

The SyclInterface library responds to `DPCTL_VERBOSITY` environment variable that controls the severity level of errors printed to console.
One can specify one of the following severity levels (in increasing order of severity): `warning` and `error`.
The `libsyclinterface` library responds to the `DPCTL_VERBOSITY` environment variable that controls the severity level of errors printed to the console.
Specify one of the following severity levels (in increasing order of severity): `warning` and `error` by running:

```bash
export DPCTL_VERBOSITY=warning
```

Messages of a given severity are shown not only in the console for that severity, but also for the higher severity. For example, the severity level `warning` will output severity errors for `error` and `warning` to the console.
Messages of a given severity are shown not only in the console for that severity but also for the higher severity. For example, the severity level `warning` outputs severity errors for `error` and `warning` to the console.

### Optional use of the Google logging library (glog)

Dpctl's error handler for libsyclinterface can be optionally configured to use [glog](https://github.com/google/glog). To use glog, follow the following steps:
The dpctl error handler for libsyclinterface can be optionally configured to use [glog](https://github.com/google/glog).

1. Install glog package of the latest version (0.5.0)
To use glog, complete the following steps:

1. Install glog package of the latest version

```bash
conda install glog
```
2. Build dpctl with glog support
2. Build dpctl with the glog support

```bash
python scripts/build_locally.py --oneapi --glog
```

3. Use `dpctl._diagnostics.syclinterface_diagnostics(verbosity="warning", log_dir=None)` context manager to switch library diagnostics on for a block of Python code.
Use `DPCTLService_InitLogger` and `DPCTLService_ShutdownLogger` library C functions during library development to initialize the Google's logging library and de-initialize accordingly
3. Use the `dpctl._diagnostics.syclinterface_diagnostics(verbosity="warning", log_dir=None)` context manager to switch library diagnostics on for a block of a Python code.
Use `DPCTLService_InitLogger` and `DPCTLService_ShutdownLogger` library C functions during library development to initialize Google's logging library and de-initialize accordingly:

```python
from dpctl._diagnostics import syclinterface_diagnostics
Expand All @@ -245,10 +272,9 @@ with syclinterface_diagnostics():
DPCTLService_InitLogger(const char *app_name, const char *log_dir);
DPCTLService_ShutdownLogger();
```

- `*app_name` - name of the executable file (prefix for logs of various levels).
- `*log_dir` - directory path for writing log files. Specifying `NULL` results in logging to ``std::cerr``.
Where:
- `*app_name` - the name of the executable file (prefix for logs of various levels).
- `*log_dir` - a directory path for writing log files. Specifying `NULL` results in logging to ``std::cerr``.

> **_NOTE:_**
>
> If `InitGoogleLogging` is not called before first use of glog, the library will self-initialize to `logtostderr` mode and log files will not be generated.
> If `InitGoogleLogging` is not called before the first use of the glog, the library self-initializes to the `logtostderr` mode, and log files are not generated.
Loading