sungwy commented on code in PR #1091:
URL: https://github.com/apache/iceberg-python/pull/1091#discussion_r1729651357
##########
Makefile:
##########
@@ -15,28 +15,37 @@
# specific language governing permissions and limitations
# under the License.
-install-poetry:
- pip install poetry==1.8.3
-install-dependencies:
- poetry install -E pyarrow -E hive -E s3fs -E glue -E adlfs -E duckdb -E
ray -E sql-postgres -E gcsfs -E sql-sqlite -E daft
+help: ## Display this help
+ @awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m\033[0m\n"}
/^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/
{ printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
Review Comment:
nit:
```suggestion
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m\033[0m\n"}
/^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-20s\033[0m %s\n", $$1, $$2 } /^##@/
{ printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
```
so that we can vertically align the starting position of the help
descriptions from
```
Usage:
make
help Display this help
install-poetry Install poetry if the user has not done that yet.
install-dependencies Install dependencies including dev and all extras
check-license Check license headers
lint lint
test Run all unit tests, can add arguments with
PYTEST_ARGS="-vv"
test-integration run all integration tests with potentially arguments
like PYTEST_ARGS="-vv"
test-adlfs Run tests marked with adlfs, can add arguments with
PYTEST_ARGS="-vv"
test-gcs Run tests marked with gcs, can add arguments with
PYTEST_ARGS="-vv"
test-coverage Run all tests with coverage including unit and
integration tests
clean clean up the project Python working environment
```
to:
```
Usage:
make
help Display this help
install-poetry Install poetry if the user has not done that yet.
install-dependencies Install dependencies including dev and all extras
check-license Check license headers
lint lint
test Run all unit tests, can add arguments with
PYTEST_ARGS="-vv"
test-integration run all integration tests with potentially arguments
like PYTEST_ARGS="-vv"
test-adlfs Run tests marked with adlfs, can add arguments with
PYTEST_ARGS="-vv"
test-gcs Run tests marked with gcs, can add arguments with
PYTEST_ARGS="-vv"
test-coverage Run all tests with coverage including unit and
integration tests
clean clean up the project Python working environment
```
##########
Makefile:
##########
@@ -72,14 +81,14 @@ test-coverage-integration:
docker compose -f dev/docker-compose-integration.yml exec -T
spark-iceberg ipython ./provision.py
poetry run coverage run --source=pyiceberg/
--data-file=.coverage.integration -m pytest tests/ -v -m integration
${PYTEST_ARGS}
-test-coverage: | test-coverage-unit test-coverage-integration
+test-coverage: | test-coverage-unit test-coverage-integration ## Run all tests
with coverage including unit and integration tests
poetry run coverage combine .coverage.unit .coverage.integration
poetry run coverage report -m --fail-under=90
poetry run coverage html
poetry run coverage xml
-clean:
+clean: ## clean up the project Python working environment
Review Comment:
nit: capitalize
```suggestion
clean: ## Clean up the project Python working environment
```
##########
Makefile:
##########
@@ -15,28 +15,37 @@
# specific language governing permissions and limitations
# under the License.
-install-poetry:
- pip install poetry==1.8.3
-install-dependencies:
- poetry install -E pyarrow -E hive -E s3fs -E glue -E adlfs -E duckdb -E
ray -E sql-postgres -E gcsfs -E sql-sqlite -E daft
+help: ## Display this help
+ @awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m\033[0m\n"}
/^[a-zA-Z_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/
{ printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
+
+install-poetry: ## Install poetry if the user has not done that yet.
+ @if ! command -v poetry &> /dev/null; then \
+ echo "Poetry could not be found. Installing..."; \
+ pip install --user poetry==1.8.3; \
+ else \
+ echo "Poetry is already installed."; \
+ fi
+
+install-dependencies: ## Install dependencies including dev and all extras
+ poetry install --all-extras
install: | install-poetry install-dependencies
-check-license:
+check-license: ## Check license headers
./dev/check-license
-lint:
+lint: ## lint
poetry run pre-commit run --all-files
-test:
+test: ## Run all unit tests, can add arguments with PYTEST_ARGS="-vv"
poetry run pytest tests/ -m "(unmarked or parametrize) and not
integration" ${PYTEST_ARGS}
-test-s3:
+test-s3: # Run tests marked with s3, can add arguments with PYTEST_ARGS="-vv"
sh ./dev/run-minio.sh
poetry run pytest tests/ -m s3 ${PYTEST_ARGS}
-test-integration:
+test-integration: ## run all integration tests with potentially arguments like
PYTEST_ARGS="-vv"
Review Comment:
nit: capitalize, and unify `PYTEST_ARGS` usage description with the other
comments
```suggestion
test-integration: ## Run all integration tests, can add arguments with
PYTEST_ARGS="-vv"
```
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
