fix: app_version#290
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a version management system using a Makefile and an APP_VERSION file to automate and verify version updates across documentation files, which have been updated to version v2.8.0. Feedback on the changes highlights cross-platform compatibility issues with sed in the Makefile, the need for robust validation of the version variable V and APP_VERSION to prevent silent failures, and formatting improvements to remove consecutive blank lines in the markdown files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| APP_VERSION_FILE := APP_VERSION | ||
| APP_VERSION := $(shell cat $(APP_VERSION_FILE) | tr -d '[:space:]') | ||
|
|
There was a problem hiding this comment.
Using sed -i '' is compatible with BSD sed (macOS) but will fail on GNU sed (Linux), which is typically used in CI/CD environments. To make the Makefile cross-platform, detect the OS and define a compatible sed in-place variable.
APP_VERSION_FILE := APP_VERSION
APP_VERSION := $(shell cat $(APP_VERSION_FILE) | tr -d '[:space:]')
# Detect OS for cross-platform sed compatibility
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S),Darwin)
SED_I := sed -i ''
else
SED_I := sed -i
endif
References
- Makefiles should be cross-platform and compatible with both BSD (macOS) and GNU (Linux) utilities, especially for standard tools like sed.
| bump-version: | ||
| ifndef V | ||
| $(error Usage: make bump-version V=v2.9.0) | ||
| endif |
There was a problem hiding this comment.
Using ifndef V only checks if the variable V is defined, but it does not catch cases where V is defined as empty (e.g., make bump-version V=). If run with an empty V, it will write an empty string to the version file and corrupt the documentation version strings. Additionally, enforcing that V starts with v (e.g., v2.9.0) prevents breaking the regex replacements in subsequent runs.
bump-version:
ifeq ($(strip $(V)),)
$(error Usage: make bump-version V=v2.9.0)
endif
ifeq ($(filter v%,$(V)),)
$(error Version V must start with 'v', e.g., V=v2.9.0)
endif
| sed -i '' 's|swanlab-server:v[0-9][0-9.]*|swanlab-server:$(NEW_V)|g' "$$f"; \ | ||
| sed -i '' 's|swanlab-house:v[0-9][0-9.]*|swanlab-house:$(NEW_V)|g' "$$f"; \ | ||
| sed -i '' 's|swanlab-cloud:v[0-9][0-9.]*|swanlab-cloud:$(NEW_V)|g' "$$f"; \ | ||
| sed -i '' 's|swanlab-next:v[0-9][0-9.]*|swanlab-next:$(NEW_V)|g' "$$f"; \ | ||
| sed -i '' 's|Self-Hosted Docker v[0-9][0-9.]*|Self-Hosted Docker $(NEW_V)|g' "$$f"; \ | ||
| sed -i '' 's|\*\*当前APP_VERSION: v[0-9][0-9.]*\*\*|**当前APP_VERSION: $(NEW_V)**|g' "$$f"; \ | ||
| sed -i '' 's|\*\*Current APP_VERSION: v[0-9][0-9.]*\*\*|**Current APP_VERSION: $(NEW_V)**|g' "$$f"; \ |
There was a problem hiding this comment.
Use the cross-platform $(SED_I) variable instead of hardcoded sed -i '' to ensure compatibility with both macOS and Linux.
$(SED_I) 's|swanlab-server:v[0-9][0-9.]*|swanlab-server:$(NEW_V)|g' "$$f"; \
$(SED_I) 's|swanlab-house:v[0-9][0-9.]*|swanlab-house:$(NEW_V)|g' "$$f"; \
$(SED_I) 's|swanlab-cloud:v[0-9][0-9.]*|swanlab-cloud:$(NEW_V)|g' "$$f"; \
$(SED_I) 's|swanlab-next:v[0-9][0-9.]*|swanlab-next:$(NEW_V)|g' "$$f"; \
$(SED_I) 's|Self-Hosted Docker v[0-9][0-9.]*|Self-Hosted Docker $(NEW_V)|g' "$$f"; \
$(SED_I) 's|\*\*当前APP_VERSION: v[0-9][0-9.]*\*\*|**当前APP_VERSION: $(NEW_V)**|g' "$$f"; \
$(SED_I) 's|\*\*Current APP_VERSION: v[0-9][0-9.]*\*\*|**Current APP_VERSION: $(NEW_V)**|g' "$$f"; \
| verify-version: | ||
| @echo "🔍 Verifying docs match APP_VERSION=$(APP_VERSION)..." | ||
| @FAILED=0; \ |
There was a problem hiding this comment.
If APP_VERSION is empty (e.g., if the APP_VERSION file is empty or missing), grep -v "$(APP_VERSION)" will evaluate to grep -v "", which filters out all lines. This causes the verification to silently pass even if there are mismatches. Adding a check to ensure APP_VERSION is not empty prevents this silent failure.
verify-version:
ifeq ($(strip $(APP_VERSION)),)
$(error APP_VERSION is empty or could not be read from $(APP_VERSION_FILE))
endif
@echo "🔍 Verifying docs match APP_VERSION=$(APP_VERSION)..."
@FAILED=0; \
| # Deploying with Kubernetes | ||
|
|
||
|
|
||
|
|
||
| > If you need to migrate from the Docker version to the Kubernetes version, please refer to [this document](/self_host/docker/migration-docker-kubernetes.md). |
There was a problem hiding this comment.
There are multiple consecutive blank lines here. It is best practice to keep a single blank line between headers and blockquotes to maintain clean markdown formatting.
| # Deploying with Kubernetes | |
| > If you need to migrate from the Docker version to the Kubernetes version, please refer to [this document](/self_host/docker/migration-docker-kubernetes.md). | |
| # Deploying with Kubernetes | |
| > If you need to migrate from the Docker version to the Kubernetes version, please refer to [this document](/self_host/docker/migration-docker-kubernetes.md). |
References
- Markdown Lint (MD012) recommends avoiding multiple consecutive blank lines to keep the document clean and consistent. (link)
| # 使用Kubernetes进行部署 | ||
|
|
||
|
|
||
|
|
||
| > 如需要从Docker版本迁移至Kubernetes版本，请参考[此文档](/self_host/docker/migration-docker-kubernetes.md)。 |
There was a problem hiding this comment.
There are multiple consecutive blank lines here. It is best practice to keep a single blank line between headers and blockquotes to maintain clean markdown formatting.
| # 使用Kubernetes进行部署 | |
| > 如需要从Docker版本迁移至Kubernetes版本，请参考[此文档](/self_host/docker/migration-docker-kubernetes.md)。 | |
| # 使用Kubernetes进行部署 | |
| > 如需要从Docker版本迁移至Kubernetes版本，请参考[此文档](/self_host/docker/migration-docker-kubernetes.md). |
References
- Markdown Lint (MD012) recommends avoiding multiple consecutive blank lines to keep the document clean and consistent. (link)
1 participant
No description provided.