Copilot commented on code in PR #839: URL: https://github.com/apache/incubator-graphar/pull/839#discussion_r2780335191
########## cpp/README.md: ########## @@ -91,7 +91,7 @@ make -j8 # if you have 8 CPU cores, otherwise adjust, use -j`nproc` for al After building, you can run the unit tests with: ```bash -git clone https://github.com/apache/incubator-graphar-testing.git testing # download the testing data +git submodule update --init --recursive # download the testing data GAR_TEST_DATA=${PWD}/testing ctest ``` Review Comment: `git submodule update --init --recursive` will check out the `testing/` submodule at the repository root, not inside the current build directory. As written, `GAR_TEST_DATA=${PWD}/testing` will point at a non-existent path when run from `cpp/build-*`, so `ctest` won’t find the data. Update the instructions to use an absolute path to the repo-root `testing/` directory (or instruct running the submodule command and setting `GAR_TEST_DATA` from the repo root), and keep the later example/benchmark commands consistent with that path. ########## CONTRIBUTING.md: ########## @@ -50,6 +50,18 @@ like (where issue \#42 is the ticket you're working on): ```shell $ git checkout -b 42-add-chinese-translations ``` +### Linting + +Install the python package `pre-commit` and run once `pre-commit install`. + +``` +pip install pre-commit +pre-commit install +pre-commit run # check staged files +pre-commit run -a # check all files +``` Review Comment: The fenced command block under “Linting” omits a language tag, while nearby command snippets use ```shell. Consider marking this block as shell as well for consistency and readability. ########## CONTRIBUTING.md: ########## @@ -50,6 +50,18 @@ like (where issue \#42 is the ticket you're working on): ```shell $ git checkout -b 42-add-chinese-translations ``` +### Linting + +Install the python package `pre-commit` and run once `pre-commit install`. + +``` +pip install pre-commit +pre-commit install +pre-commit run # check staged files +pre-commit run -a # check all files +``` + +This will setup a git pre-commit-hook that is executed on each commit and will report the linting problems. Review Comment: Minor wording/formatting: “This will setup …” should be “This will set up …” (or “This sets up …”), and there is trailing whitespace at the end of the line. Fixing this avoids typos being flagged by linters. ```suggestion This will set up a git pre-commit-hook that is executed on each commit and will report the linting problems. ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
