Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions .github/workflows/deploy.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

Image Why are we getting 3.4.0 version? This doesnt seem correct

Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,8 @@ jobs:
version: ${{ steps.versionextractor.outputs.version }}
commit: ${{ steps.lastcommit.outputs.commit }}
proceed: ${{ steps.skipcondition.outputs.proceed }}
package_name: ${{ steps.changedpackage.outputs.package_name }}
package_version: ${{ steps.changedpackage.outputs.package_version }}

steps:
- name: Checkout Project
Expand Down Expand Up @@ -318,6 +320,23 @@ jobs:
echo "message=$(git log -1 --oneline --no-decorate)" >> $GITHUB_OUTPUT
echo "version=v$(yarn package-tools sync --tag ${GITHUB_REF##*/} --packages webex | awk '{print $3}' | tr -d '%')" >> $GITHUB_OUTPUT

- name: Extract changed package version
Copy link
Contributor

Choose a reason for hiding this comment

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

How have we tested this? In your testing run this step (publish-tag) didnt even run.

Image

id: changedpackage
run: |
PACKAGES="${{ needs.generate-package-matrix.outputs.node-recursive }}"
CLEAN=$(echo "$PACKAGES" | tr -d '{}')
if echo "$CLEAN" | tr ',' '\n' | grep -q "^webex$"; then
PKG="webex"
else
PKG=$(echo "$CLEAN" | tr ',' '\n' | head -1 | tr -d ' ')

Choose a reason for hiding this comment

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

P1 Badge Parse node-recursive package list using whitespace separators

needs.generate-package-matrix.outputs.node-recursive comes from yarn package-tools list --mode node, which emits package names space-delimited, but this step treats the value as comma-delimited and then strips spaces; for multi-package outputs like webex @webex/calling, PKG becomes webex@webex/calling. That invalid package name causes yarn package-tools sync --packages $PKG to resolve no workspace and produce an empty version, so downstream PR comments are posted with package_version set to just v and broken release/changelog links.

Useful? React with 👍 / 👎.

fi
if [ -n "$PKG" ]; then
PKG_VERSION=$(yarn package-tools sync --tag ${GITHUB_REF##*/} --packages $PKG | awk '{print $3}' | tr -d '%')
echo "package_name=${PKG}" >> $GITHUB_OUTPUT
echo "package_version=v${PKG_VERSION}" >> $GITHUB_OUTPUT
echo "📦 Selected package: ${PKG} version: ${PKG_VERSION}"
fi

- name: Conditions to skip tag publish
id: skipcondition
run: |
Expand Down Expand Up @@ -365,3 +384,92 @@ jobs:
else
git push origin ${{ steps.versionextractor.outputs.version }}
fi

comment-on-prs:
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have everything we need to comment on PRs in deploy.yml now, why do we need pr-comment-bot.yml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we have everything we need to comment on PRs in deploy.yml now, why do we need pr-comment-bot.yml?

@mkesavan13 You're right, we don't need it anymore. I'll delete pr-comment-bot.yml since the logic has moved into the Deploy CD pipeline.

name: Comment on Merged PR
needs: [publish-tag, generate-package-matrix]
runs-on: ubuntu-latest
if: ${{ needs.generate-package-matrix.outputs.node-recursive != '' }}

steps:
- name: Checkout Project
uses: actions/checkout@v3

- name: Get Pull Request Number
id: pr
run: echo "pull_request_number=$(gh pr view --json number -q .number || echo "")" >> $GITHUB_OUTPUT

Choose a reason for hiding this comment

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

P1 Badge Select merged PR by SHA instead of branch default

This job runs on push to next, but gh pr view --json number -q .number is invoked without a PR/branch argument, so it resolves the PR for the currently checked-out branch (next) rather than the PR that introduced the pushed commit. In typical merge-to-next runs there is no PR for next, so pull_request_number is empty and the comment step is skipped entirely; if a long-lived PR from next exists, the bot can comment on that unrelated PR instead. Resolve PRs from the push commit SHA (or merge metadata) so comments target the released PR reliably.

Useful? React with 👍 / 👎.

env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Post Comment on PR
if: ${{ steps.pr.outputs.pull_request_number != '' }}
uses: actions/github-script@v7
env:
INPUT_VERSION: ${{ needs.publish-tag.outputs.package_version }}
INPUT_PACKAGES: ${{ needs.generate-package-matrix.outputs.node-recursive }}
INPUT_PR_NUMBER: ${{ steps.pr.outputs.pull_request_number }}
INPUT_PACKAGE_NAME: ${{ needs.publish-tag.outputs.package_name }}
with:
script: |
const version = process.env.INPUT_VERSION || '';
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not how we use the env variables in actions. please refer to 'GIT_AUTHOR_NAME' in this file to understand the usage

const versionNumber = version.replace(/^v/, '');
const prNumber = parseInt(process.env.INPUT_PR_NUMBER);

if (!prNumber) {
console.log('No PR number found, skipping');
return;
}

console.log(`Posting comment on PR #${prNumber} for version ${version}`);

const existingComments = await github.rest.issues.listComments({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: prNumber
});
Comment on lines +517 to +521

Choose a reason for hiding this comment

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

P2 Badge Paginate comment lookup before duplicate-checking release note

The duplicate-check only inspects the single page returned by issues.listComments, but that API returns 30 comments by default. On PRs with more comments, an existing bot release comment can be outside this page, so reruns post duplicate “Your changes are now available” messages. Use github.paginate (or explicit paging) when searching existing comments.

Useful? React with 👍 / 👎.

Comment on lines +517 to +521

Choose a reason for hiding this comment

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

P1 Badge Catch GitHub API failures in PR comment step

This script performs issues.listComments/issues.createComment without any error handling, so transient GitHub API failures (rate limiting, token permission changes, or temporary API errors) will throw and fail the entire Deploy CD workflow even though package publish/tagging has already completed. Because this comment job now runs inline in deploy, a non-critical notification failure can incorrectly mark production deploys as failed; wrap the API section in try/catch (or continue-on-error) to keep deploy status reliable.

Useful? React with 👍 / 👎.


const alreadyCommented = existingComments.data.some(c =>
c.user.type === 'Bot' &&
c.body.includes('Your changes are now available') &&
c.body.includes(version)
);

if (alreadyCommented) {
console.log(`Already commented on PR #${prNumber} for ${version}`);
return;
}
Comment on lines +529 to +532
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkesavan13 This is a safety check to prevent duplicate comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a scenario where duplicate commenting is expected?


const packagesRaw = process.env.INPUT_PACKAGES || '';
const packages = packagesRaw
.replace(/[{}]/g, '')
.split(',')
Comment on lines +535 to +537

Choose a reason for hiding this comment

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

P2 Badge Split node-mode package output on whitespace

INPUT_PACKAGES here is wired to needs.generate-package-matrix.outputs.node-recursive, which is produced by yarn package-tools list --mode node and therefore emits package names separated by spaces (see packages/tools/package/src/commands/list/list.ts, packageNames.join(' ')). Splitting only on commas turns multi-package outputs like webex @webex/calling into a single entry, so packages.includes('webex') fails and both the "Packages updated" text and changelog package query can be wrong for normal multi-package releases.

Useful? React with 👍 / 👎.

.map(p => p.trim())
.filter(Boolean);

const primaryPackage = process.env.INPUT_PACKAGE_NAME || 'webex';
const packageList = packages.slice(0, 5).map(p => `\`${p}\``).join(', ');
const morePackages = packages.length > 5 ? ` and ${packages.length - 5} more` : '';

const stableVersion = versionNumber.replace(/-next\..*$/, '');
const changelogUrl = `https://web-sdk.webex.com/changelog/?stable_version=${stableVersion}&package=${primaryPackage}&version=${versionNumber}`;
Comment on lines +545 to +546
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this fix the problem we had of choosing packages[0] while version number was different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated the approach. Instead of hardcoding --packages webex for the version, I added a new changedpackage step in publish-tag that first picks the package (prefers webex if it's in the changed list, otherwise uses the first changed package), then fetches that same package's version from NPM. Both package_name and package_version are derived from the same variable ($PKG), so the version always corresponds to the selected package. This eliminates the mismatch we had before where the version came from webex but the package could be something else.


const commentBody = [
'## 🎉 Your changes are now available!',
'',
`**Released in:** [\`${version}\`](https://github.com/${context.repo.owner}/${context.repo.repo}/releases/tag/${version})`,
`**Packages updated:** ${packageList}${morePackages}`,
'',
`📖 **[View full changelog →](${changelogUrl})**`,
'',
'---',
`<sub>🤖 This is an automated message.</sub>`
].join('\n');

await github.rest.issues.createComment({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: prNumber,
body: commentBody
});

console.log(`✅ Commented on PR #${prNumber}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove some of the console.logs.

257 changes: 0 additions & 257 deletions .github/workflows/pr-comment-bot.yml

This file was deleted.

Loading