Skip to content

Conversation

@anxolin
Copy link
Contributor

@anxolin anxolin commented Sep 26, 2025

Attempt to fix the name used in the main branch when publishing packages to GH packages.

image

This is a follow up on: #539

The goal is to use <version>-main-<hash>

Summary by CodeRabbit

  • Chores

    • Publishing workflow now always creates pre-release packages and restores original versions in every run, providing consistent pre-release builds.
    • Users may see more frequent pre-release packages available for early testing; no change to app features or APIs.
  • Tests

    • Previously skipped unit test suites have been re-enabled, restoring fuller test execution coverage. No functional behavior or public API changes.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 26, 2025

Walkthrough

Removed PR-only conditionals from two steps in the publish GitHub Actions workflow so they run unconditionally, and re-enabled previously skipped unit tests in the BridgingSdk test suites for BestQuoteStrategy and MultiQuoteStrategy.

Changes

Cohort / File(s) Change Summary
Workflow step gating removal
.github/workflows/publish-github-packages.yml
Removed if: steps.version.outputs.is_pr == 'true' from two steps and renamed them: "Create pre-release versions for PRs" → "Create pre-release versions" and "Restore original versions (PR only)" → "Restore original versions". Those steps now run unconditionally; later steps still reference is_pr outputs.
Re-enabled Bridging tests
packages/bridging/src/BridgingSdk/strategies/BestQuoteStrategy.test.ts, packages/bridging/src/BridgingSdk/strategies/MultiQuoteStrategy.test.ts
Removed skipped (.skip) modifiers from describe blocks to re-enable tests; minor formatting of Promise-based delays retained without behavioral change. No functional code or exported API changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant GH as GitHub Actions
  participant Job as publish-github-packages

  Dev->>GH: Push / PR event
  GH->>Job: Start workflow
  Job->>Job: Determine is_pr (steps.version.outputs.is_pr)

  rect rgba(220,235,255,0.25)
    Note over Job: Previous (gated) flow
    alt is_pr == 'true'
      Job-->Job: Create pre-release versions (PR-only)
      Job-->Job: Restore original versions (PR-only)
    else
      Job-->Job: Skip those steps
    end
  end

  rect rgba(220,255,220,0.25)
    Note over Job: New (ungated) flow
    Job-->Job: Create pre-release versions (always)
    Job-->Job: Restore original versions (always)
  end

  Job-->GH: Continue remaining steps (some still reference is_pr)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

allow-publish

Suggested reviewers

  • alfetopito

Poem

I hop through CI with tiny paws,
Gates lifted now — applause, applause.
Tests awaken, run with cheer,
Pre-releases spring when they appear.
A rabbit's nibble, tidy and bright,
Packages hopping into the night. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The pull request title refers to renaming packages with a better format, but the actual changes shown remove gating conditions from the publish workflow and re-enable skipped tests rather than implement the described naming format for main branch packages. This mismatch means the title does not accurately summarize the primary modifications made. Please update the title to describe the actual changes—removal of PR-specific workflow gating and reactivation of tests—or modify the PR to include the intended package renaming logic so that the title accurately reflects the content of the changeset.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch auto-publish-registy-main

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

📦 GitHub Packages Published

Last updated: Sep 26, 2025, 04:59:47 PM UTC

The following packages have been published to GitHub Packages with pre-release version pr-543-9a887eda:


Installation

These packages require authentication to install from GitHub Packages. First, configure your token:

# Set your GitHub token
#  1. create one at https://github.com/settings/tokens. Make sure you select the option "Generate new token (classic)"
#  2. Check only the "read:packages" scope
#  3. Add the token to the npm config
npm config set //npm.pkg.github.com/:_authToken YOUR_GITHUB_TOKEN

Then install any of the packages above:

# Yarn
yarn add @cowprotocol/[email protected] --registry https://npm.pkg.github.com

# pnpm
pnpm install @cowprotocol/[email protected] --registry https://npm.pkg.github.com

# NPM
npm install @cowprotocol/[email protected] --registry https://npm.pkg.github.com

View Packages

You can view the published packages at: https://github.com/cowprotocol/cow-sdk/packages

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/bridging/src/BridgingSdk/strategies/MultiQuoteStrategy.test.ts (1)

28-28: Add afterEach to restore real timers

Several tests inside this block switch to fake timers and rely on manually calling jest.useRealTimers() at the end. If any of those assertions fail early, the suite will leak fake timers into subsequent tests, causing hard-to-track flakiness. Please add an afterEach guard to always restore real timers.

 describe(`MultiQuoteStrategy with ${adapterName}`, () => {
+    afterEach(() => {
+      jest.useRealTimers()
+    })
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 307225d and 5371593.

📒 Files selected for processing (2)
  • packages/bridging/src/BridgingSdk/strategies/BestQuoteStrategy.test.ts (2 hunks)
  • packages/bridging/src/BridgingSdk/strategies/MultiQuoteStrategy.test.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/bridging/src/BridgingSdk/strategies/MultiQuoteStrategy.test.ts (1)
packages/bridging/src/BridgingSdk/mock/bridgeRequestMocks.ts (1)
  • bridgeQuoteResult (127-161)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: eslint
  • GitHub Check: Publish to GitHub Packages
  • GitHub Check: test
  • GitHub Check: Build Package
🔇 Additional comments (3)
packages/bridging/src/BridgingSdk/strategies/BestQuoteStrategy.test.ts (2)

27-27: Re-enabling BestQuoteStrategy adapter suite looks good

Glad to see this suite running again for every adapter—no concerns here.


125-125: Happy to see the execute suite back in rotation

Re-enabling the core execution flow tests should help catch regressions early; LGTM.

packages/bridging/src/BridgingSdk/strategies/MultiQuoteStrategy.test.ts (1)

255-275: Async delay mocks remain correct

The expanded promise bodies keep the original behavior while making the resolution logic easier to read—looks good.

@anxolin anxolin merged commit 3c57f55 into main Sep 29, 2025
8 of 9 checks passed
@anxolin anxolin deleted the auto-publish-registy-main branch September 29, 2025 10:53
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants