wp-parsely icon indicating copy to clipboard operation
wp-parsely copied to clipboard
verified publisher icon Parsely

Improve release workflow

Open alecgeatches opened this issue 8 months ago • 1 comments

Description

WIP. Addresses the release workflow issues outlined in https://github.com/Parsely/wp-parsely/issues/2938:

  • [x] Generated PR has an empty space before the first line of the changelog ( ### Added)
  • [x] Generated PR hasn't a set milesone
  • [ ] GitHub action bot not signing commits / PRs (we might need our own bot user for that)
  • [x] When running with dry-run, if the tag already exists it is deleted locally, but when pushing the tag it fails since it already exists on the remote.
  • [ ] GitHub release is not published automatically when the zip file is uploaded

Nice to have / Improvements

  • [ ] When running the release workflow without dry-run set, it might optionally attempt to remove any old release and tag created by the dry-run execution.
  • [ ] When the Update version number and changelog for x.x.x release PR is merged, maybe trigger automatically an action that creates a Release wp-parsely x.x.x PR that merges develop into trunk.
  • [ ] After the release, automatically create a Merge trunk into develop after the wp-parsely x.x.x release PR.

Motivation and context

How has this been tested?

alecgeatches avatar Apr 10 '25 21:04 alecgeatches

📝 Walkthrough 
## Walkthrough

This pull request updates two GitHub workflow files. In the bump-version workflow, the processing of the `PARSELY_RELEASE_LOG` variable has been enhanced by appending a `sed` command to remove leading whitespace. Additionally, the `gh pr create` command now includes a `--milestone` parameter to associate the pull request with the new version's milestone. In the release-plugin workflow, changes improve error handling for tag deletions by clarifying comments and adding a command to remove remote tags if they exist.

## Changes

| File                                        | Change Summary                                                                                                                                                      |
|---------------------------------------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `.github/workflows/bump-version.yml`        | Modified the processing of `PARSELY_RELEASE_LOG` to strip leading whitespace using an additional `sed` command; added `--milestone ${{ env.NEW_VERSION }}` to the `gh pr create` command. |
| `.github/workflows/release-plugin.yml`      | Enhanced error handling by updating comments and adding the command `git push --delete origin ":refs/tags/${TAG_NAME}" || true` to ensure deletion of remote tags.                      |
| `src/content-helper/editor-sidebar/class-editor-sidebar.php` | Updated PHPDoc annotation for `$post` variable to `WP_Post|null` and added explicit null check before calling `Dashboard_Link::can_show_link` in `get_parsely_post_url` method. |

## Sequence Diagram(s)

```mermaid
sequenceDiagram
    participant Runner as GitHub Workflow Runner
    participant Env as Environment Variables
    participant Bump as Bump-Version Script
    participant GH as GitHub CLI

    Runner->>Bump: Trigger bump version workflow
    Bump->>Env: Retrieve PARSELY_RELEASE_LOG
    Bump->>Bump: Apply sed to remove leading whitespace
    Bump->>GH: Execute "gh pr create" with --milestone parameter
    GH-->>Bump: Pull request created with milestone

sequenceDiagram
    participant Runner as GitHub Workflow Runner
    participant Git as Git Commands
    participant Remote as Remote Repository

    Runner->>Git: Check for existing tag locally and remotely
    Git->>Git: Delete local tag if it exists
    Git->>Remote: Execute "git push --delete origin ':refs/tags/${TAG_NAME}'" to delete remote tag
    Remote-->>Git: Confirm tag deletion or return error status
    Git-->>Runner: Tag deletion outcome handled (errors suppressed)

Possibly related issues

  • Parsely/wp-parsely#2938: The modifications to the PARSELY_RELEASE_LOG formatting and the addition of a milestone parameter directly align with the improvements requested in this issue regarding changelog formatting and pull request management.

Possibly related PRs

  • Parsely/wp-parsely#2923: The enhancements made in the bump-version workflow for changelog formatting and milestone association complement the adjustments introduced in this PR.

Suggested labels

Maintenance, Changelog: Added

Suggested reviewers

  • vaurdan

</details>

<!-- walkthrough_end -->

<!-- announcements_start -->

> [!TIP]
> <details>
> <summary>⚡💬 Agentic Chat (Pro Plan, General Availability)</summary>
> 
> - We're introducing multi-step agentic chat in review comments and issue comments, within and outside of PR's. This feature enhances review and issue discussions with the CodeRabbit agentic chat by enabling advanced interactions, including the ability to create pull requests directly from comments and add commits to existing pull requests.
> 
> </details>

<!-- announcements_end -->

---

<details>
<summary>📜 Recent review details</summary>

**Configuration used: .coderabbit.yaml**
**Review profile: CHILL**
**Plan: Pro**

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between ddd02cae3e9f231b141c5c135bd22edc0c98dd8a and 0686c323f36f616ba99710bab6399a185f2a5cf7.

</details>

<details>
<summary>📒 Files selected for processing (1)</summary>

* `src/content-helper/editor-sidebar/class-editor-sidebar.php` (1 hunks)

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>📓 Path-based instructions (1)</summary>

<details>
<summary>`**/*.{html,php}`: "Perform a detailed review of the provided code with following key aspects in mind:
  - Review the HTML and PHP code to ensure it is well-structured and adheres ...</summary>

> `**/*.{html,php}`: "Perform a detailed review of the provided code with following key aspects in mind:
>   - Review the HTML and PHP code to ensure it is well-structured and adheres to best practices.
>   - Ensure the code follows WordPress coding standards and is well-documented.
>   - Confirm the code is secure and free from vulnerabilities.
>   - Optimize the code for performance, removing any unnecessary elements.
>   - Validate comments for accuracy, currency, and adherence to WordPress coding standards.
>   - Ensure each line comment concludes with a period.
>   - Verify code compatibility with the latest version of WordPress, avoiding deprecated functions or features."
> 

- `src/content-helper/editor-sidebar/class-editor-sidebar.php`

</details>

</details><details>
<summary>🧬 Code Graph Analysis (1)</summary>

<details>
<summary>src/content-helper/editor-sidebar/class-editor-sidebar.php (2)</summary><blockquote>

<details>
<summary>tests/Integration/TestCase.php (1)</summary>

* `get_post` (249-264)

</details>
<details>
<summary>src/class-dashboard-link.php (2)</summary>

* `Dashboard_Link` (21-82)
* `can_show_link` (79-81)

</details>

</blockquote></details>

</details>

</details>

<details>
<summary>⏰ Context from checks skipped due to timeout of 90000ms (3)</summary>

* GitHub Check: Analyze (javascript)
* GitHub Check: build
* GitHub Check: E2E against WordPress latest

</details>

<details>
<summary>🔇 Additional comments (2)</summary><blockquote>

<details>
<summary>src/content-helper/editor-sidebar/class-editor-sidebar.php (2)</summary>

`109-109`: **Improved type annotation accuracy for the $post variable.**

The PHPDoc annotation has been updated to accurately reflect that the `$post` variable can be `null`, which is important for type safety and documentation accuracy. This aligns with the behavior of `get_post()` which can return `null` when a post doesn't exist.

---

`113-113`: **Good defensive programming with null check before method call.**

The code now explicitly checks if `$post` is `null` before invoking `Dashboard_Link::can_show_link()`. This prevents potential PHP errors, as the referenced method expects a non-null `WP_Post` object as shown in the `Dashboard_Link` class.

```php
public static function can_show_link( WP_Post $post, Parsely $parsely ): bool

This change adds defensive programming that correctly handles the scenario where get_post() returns null, improving robustness and preventing potential runtime errors.

✨ Finishing Touches
  • [ ] 📝 Generate Docstrings
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Apr 10 '25 21:04 coderabbitai[bot]

@acicovic Howdy! This is ready for review. For now, I'm focusing on the top 5 items and ignoring the nice-to-haves. There are a couple that I haven't completed that I'd like feedback on:

GitHub action bot not signing commits / PRs (we might need our own bot user for that)

I found a couple of ways to accomplish this:

  1. Using GITHUB_TOKEN and manual API requests, see Solution 1 here and the examples on this post. This uses some wacky stuff with GitHub's API directly to commit files over HTTP. This seems unusual and different enough from rest of our git-managing process to probably not be a good solution.
  2. Using a bot, as originally suggested. This is also covered in Solution 2 on this page. The steps here would be:
    • Create a bot account and store credentials
    • Generate a PGP signature for that account and store the secrets in the repository
    • Import the PGP signature in our actions

I haven't implemented either yet, but if we want to go ahead with this I think a bot account makes the most sense. Does that sound good to you, or do you think we should keep things as-is?

GitHub release is not published automatically when the zip file is uploaded

We use ncipollo/release-action@v1 in our release workflow. From my understanding of the documentation, draft: ${{ env.DRY_RUN }} should mean that we make a published release when we're running the action outside of a dry run. Do you have any insight on how this wasn't working during the last release?

alecgeatches avatar Apr 14 '25 19:04 alecgeatches

@alecgeatches, thank you for your work.

I'll share with you internally all the context I've got about the workflow run. I'm unsure it will shed light to your questions though. To be more specific to the questions at hand:

I think a bot account makes the most sense.

I agree.

Do you have any insight on how this wasn't working during the last release?

Unfortunately I don't, and I'm unsure what would cause this. I can see that DRY_RUN is evaluated as a string and as a bool in different places, though this might be expected and not the source of the issue.

To conclude, from my understanding none of these are blocking. Thus, we could make our next release without these and see if any of them cause enough friction for us to consider fixing them for v3.19 if there's bandwidth.

acicovic avatar Apr 15 '25 09:04 acicovic

Thank you @acicovic! I'll commit as-is for now, and we can evaluate if we want the bot changes and understand the published release problem again.

alecgeatches avatar Apr 15 '25 16:04 alecgeatches