Refactor(workflow): enhance payload validation and improve documentat…#24817
Refactor(workflow): enhance payload validation and improve documentat…#24817ahmetcelik05 merged 1 commit intodevfrom
Conversation
…ion update process in update-studio-docs workflow
There was a problem hiding this comment.
Pull request overview
This PR refactors the GitHub workflow for updating ABP Studio documentation, focusing on improving payload validation, enhancing documentation formatting, and implementing a more sophisticated version mapping system.
Changes:
- Enhanced comment formatting throughout the workflow for better clarity and consistency
- Updated release notes format to include dates and improved insertion logic to place entries under "Latest" section
- Replaced simple version-mapping.md append logic with a complex Python script that manages version ranges intelligently
- Modified PR creation to capture and use PR URL instead of branch name for auto-merge
| GH_TOKEN: ${{ secrets.BOT_SECRET }} | ||
| run: | | ||
| gh pr merge "$BRANCH" --squash --auto --admin | ||
| gh pr merge "$PR_URL" --squash --auto |
There was a problem hiding this comment.
The --admin flag has been removed from the auto-merge command. If branch protection rules are in place that require admin privileges to bypass (such as required reviews or status checks), this change may cause the auto-merge to fail where it previously succeeded. Verify that this is intentional and that the workflow still functions correctly with existing branch protection rules.
| header = lines[:2] | ||
| rows = lines[2:] |
There was a problem hiding this comment.
The Python script doesn't handle the case where the version-mapping.md file exists but is empty or doesn't contain the header rows. Accessing lines[:2] will fail with an IndexError if the file has fewer than 2 lines, causing the workflow to fail unexpectedly.
| header = lines[:2] | |
| rows = lines[2:] | |
| if len(lines) >= 2: | |
| header = lines[:2] | |
| rows = lines[2:] | |
| else: | |
| header = [ | |
| "| ABP Studio Version | ABP Version |\n", | |
| "|-------------------|-------------|\n", | |
| ] | |
| rows = lines |
|
|
||
| if grep -q "## Version $VERSION" "$FILE"; then | ||
| echo "Release notes already contain $VERSION, skipping" | ||
| if grep -q "## $VERSION" "$FILE"; then |
There was a problem hiding this comment.
The grep pattern has changed from matching "## Version $VERSION" to "## $VERSION", but the new format being written is "## $VERSION ($DATE)". This means the duplicate check will fail for existing entries with dates. If a version already exists with the format "## 1.0.0 (2024-01-01)", the new grep won't match it because it's looking for exactly "## 1.0.0" without the date suffix, and a duplicate entry could be added.
| if grep -q "## $VERSION" "$FILE"; then | |
| if grep -qE "^## $VERSION " "$FILE"; then |
| python3 <<'EOF' | ||
| import re | ||
| from packaging.version import Version | ||
|
|
||
| file_path = "docs/en/studio/version-mapping.md" | ||
| studio = Version("${STUDIO_VERSION}") |
There was a problem hiding this comment.
The Python script uses string interpolation with shell variables (${STUDIO_VERSION} and ${ABP_VERSION}) inside a heredoc, but the heredoc is quoted with 'EOF' which prevents variable expansion. This means the Python script will try to parse the literal string "${STUDIO_VERSION}" instead of the actual version value, causing the script to fail.
| elif studio == end.next_patch(): | ||
| handled = True | ||
| new_rows.append(f"| {start} - {studio} | {abp} |\n") | ||
| else: | ||
| new_rows.append(row) | ||
| else: | ||
| v = Version(studio_range) | ||
| if studio == v: | ||
| handled = True | ||
| new_rows.append(row) | ||
| elif studio == v.next_patch(): |
There was a problem hiding this comment.
The Python script calls end.next_patch() and v.next_patch() on Version objects, but the packaging.version.Version class doesn't have a next_patch() method. This will result in an AttributeError when the code tries to execute these lines.
| FILE="docs/en/studio/version-mapping.md" | ||
| VERSION="${{ github.event.client_payload.version }}" | ||
| STUDIO_VERSION="${{ github.event.client_payload.version }}" | ||
| ABP_VERSION="dev" # gerekiyorsa payload’dan alabilirsin |
There was a problem hiding this comment.
The variable ABP_VERSION is hardcoded to "dev" with a comment suggesting it should come from the payload if needed. This is inconsistent with the workflow's design where other values come from the payload. If "dev" is always the correct value, the comment should be removed. If it should be dynamic, it should be extracted from the payload like other values.
| ABP_VERSION="dev" # gerekiyorsa payload’dan alabilirsin | |
| ABP_VERSION="${{ github.event.client_payload.abpVersion || 'dev' }}" |
No description provided.