Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: enable anonymous issue tracking #1685

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

feat: enable anonymous issue tracking #1685

wants to merge 12 commits into from

Conversation

Nick-1979
Copy link
Member

@Nick-1979 Nick-1979 commented Dec 2, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced error reporting toggle in settings, allowing users to enable or disable error reporting.
    • Integrated Sentry for enhanced error tracking and performance monitoring.
  • Improvements

    • Added source map generation for better debugging.
    • Updated build process to include project build step before release.
  • Version Updates

    • Updated extension version to 0.34.0.
  • Localization

    • Added new translation entry for "Enable error reporting."

Copy link
Contributor

coderabbitai bot commented Dec 2, 2024

Warning

Rate limit exceeded

@Nick-1979 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 24 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 6a46490 and fb73eee.

Walkthrough

The pull request modifies the CI workflow and several components to integrate Sentry for error tracking and performance monitoring. It adds a new step to the CI process for semantic release. Dependencies in package.json are updated, the extension's version is incremented, and Sentry is integrated into the extension's codebase, including UI changes for managing error reporting settings. These changes collectively enhance the project's release process and error tracking capabilities.

Changes

File Path Change Summary
.github/workflows/ci-workflow.yml Updated release job to include a new step titled "Run Semantic Release" using npx semantic-release.
package.json Added dependencies @sentry/react and @sentry/webpack-plugin. Updated build and start scripts to set NODE_ENV.
packages/extension/package.json Version updated from 0.33.1 to 0.34.0.
packages/extension/src/extension.ts Integrated Sentry for error tracking; added logic for enabling/disabling Sentry based on local storage.
packages/extension/src/packageInfo.ts Updated version field from '0.33.1' to '0.34.0'.
packages/extension/webpack.shared.cjs Changed devtool to 'source-map', added rules for file handling, and introduced sentryWebpackPlugin.
packages/extension-polkagate/src/partials/SettingSubMenu.tsx Added UI for toggling error reporting with a new checkbox and state management.
packages/extension/public/locales/en/translation.json Added translation entry for "Enable error reporting".

Possibly related PRs

Suggested labels

new feature

Suggested reviewers

  • AMIRKHANEF

🐇 In the garden, we hop and play,
With Sentry's help, we track the way.
Errors reported, our code shines bright,
Building and uploading, all feels right!
So let’s cheer for changes, both big and small,
Together we thrive, we’ll conquer it all! 🌼


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 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 resolve resolve all the CodeRabbit review comments.
  • @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.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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.

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: 8

🧹 Outside diff range and nitpick comments (1)
.github/workflows/ci-workflow.yml (1)

80-80: Fix trailing whitespace

Remove trailing whitespace to maintain consistent code style.

-        run: yarn build
-  
+        run: yarn build
+
         - name: Upload source maps to Sentry

Also applies to: 85-85

🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 44dcb3b and 2723ebf.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (7)
  • .github/workflows/ci-workflow.yml (1 hunks)
  • .gitignore (1 hunks)
  • package.json (1 hunks)
  • packages/extension/package.json (1 hunks)
  • packages/extension/src/extension.ts (1 hunks)
  • packages/extension/src/packageInfo.ts (1 hunks)
  • packages/extension/webpack.shared.cjs (3 hunks)
✅ Files skipped from review due to trivial changes (3)
  • .gitignore
  • packages/extension/src/packageInfo.ts
  • packages/extension/package.json
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 80-80: trailing spaces

(trailing-spaces)


[error] 85-85: trailing spaces

(trailing-spaces)

🔇 Additional comments (1)
packages/extension/webpack.shared.cjs (1)

110-115: Verify Sentry webpack plugin configuration

The Sentry webpack plugin configuration needs verification for proper source map upload.

✅ Verification successful

Sentry configuration is properly implemented

The verification shows that:

  • Source map generation is correctly enabled with devtool: 'source-map'
  • Sentry webpack plugin is properly configured and placed after other plugins
  • Sentry is correctly initialized in extension.ts with proper DSN and integrations
  • Environment variables are properly handled through dotenv-webpack
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Sentry environment configuration

# Check if SENTRY_AUTH_TOKEN is configured in .env
if [ -f ".env" ]; then
  echo "Checking .env for Sentry configuration..."
  grep -E "SENTRY_AUTH_TOKEN|SENTRY_ORG|SENTRY_PROJECT" .env
fi

# Verify webpack output directory exists
if [ -d "packages/extension/build" ]; then
  echo "Checking for source maps in build directory..."
  find packages/extension/build -name "*.map" -type f
fi

Length of output: 55


Script:

#!/bin/bash
# Let's verify the Sentry webpack plugin configuration more thoroughly

# Check webpack config for Sentry plugin
echo "Checking webpack configuration files for Sentry setup..."
rg -l "sentry" packages/extension/

# Check package.json for Sentry dependencies
echo -e "\nChecking Sentry dependencies..."
cat packages/extension/package.json | grep -i "sentry"

# Check for any environment files
echo -e "\nChecking for environment configuration files..."
fd -H "\.env" -t f
fd "\.env\." -t f

# Look for source map configuration in webpack
echo -e "\nChecking source map configuration in webpack..."
rg "devtool|sourceMap" packages/extension/webpack.*.cjs

Length of output: 926


Script:

#!/bin/bash
# Let's check the specific Sentry configuration and environment setup

# Check the example environment file for Sentry configuration
echo "Checking .env.example for Sentry configuration..."
cat .env.example | grep -i "sentry"

# Check the actual Sentry implementation in webpack.shared.cjs
echo -e "\nChecking Sentry webpack plugin implementation..."
rg -A 5 -B 5 "sentryWebpackPlugin" packages/extension/webpack.shared.cjs

# Check for any Sentry initialization in the extension
echo -e "\nChecking Sentry initialization in extension.ts..."
rg -A 5 -B 5 "Sentry" packages/extension/src/extension.ts

Length of output: 1804

packages/extension/src/extension.ts Outdated Show resolved Hide resolved
packages/extension/src/extension.ts Outdated Show resolved Hide resolved
packages/extension/src/extension.ts Outdated Show resolved Hide resolved
packages/extension/webpack.shared.cjs Show resolved Hide resolved
.github/workflows/ci-workflow.yml Outdated Show resolved Hide resolved
.github/workflows/ci-workflow.yml Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
package.json Show resolved Hide resolved
@Nick-1979 Nick-1979 requested a review from AMIRKHANEF December 6, 2024 23:09
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: 2

🧹 Outside diff range and nitpick comments (4)
packages/extension/webpack.shared.cjs (2)

Line range hint 35-45: Document the purpose of new webpack rules

The new rules are marked as "added for polkagate" but lack explanation of their purpose and necessity.

Please add comments explaining:

  • Why fullySpecified: false is needed for .m?js files
  • The specific CSS handling requirements for the project

109-118: Consider making Sentry configuration more flexible

While the Sentry integration is well-structured, consider making the organization and project names configurable via environment variables for better flexibility across different environments.

Consider this modification:

   ...(process.env.NODE_ENV === 'development'
     ? []
     : [sentryWebpackPlugin({
         authToken: process.env.SENTRY_AUTH_TOKEN,
-        org: 'polkagate',
-        project: 'javascript-react',
+        org: process.env.SENTRY_ORG || 'polkagate',
+        project: process.env.SENTRY_PROJECT || 'javascript-react',
         telemetry: false
       })]
   )
packages/extension-polkagate/src/partials/SettingSubMenu.tsx (2)

33-35: Add effect to sync localStorage changes across tabs

The current implementation doesn't handle localStorage changes from other tabs/windows.

+ useEffect(() => {
+   const handleStorageChange = (e: StorageEvent) => {
+     if (e.key === 'sentryEnabled') {
+       setIsSentryEnabled(e.newValue === 'true');
+     }
+   };
+   window.addEventListener('storage', handleStorageChange);
+   return () => window.removeEventListener('storage', handleStorageChange);
+ }, []);

96-104: Add tooltip explaining error reporting implications

Users should understand what data will be collected when enabling error reporting.

   <Grid item pt='10px' textAlign='left'>
     <Checkbox2
       checked={isSentryEnabled}
       iconStyle={{ transform: 'scale(1.13)' }}
       label={t('Enable error reporting')}
       labelStyle={{ fontSize: '17px', fontWeight: 300, marginLeft: '7px' }}
       onChange={toggleErrorReporting}
+      tooltip={t('When enabled, anonymous crash reports and performance data will be collected to help improve the extension')}
     />
   </Grid>
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between ecb800b and 664de2a.

📒 Files selected for processing (7)
  • package.json (3 hunks)
  • packages/extension-polkagate/src/partials/SettingSubMenu.tsx (5 hunks)
  • packages/extension/package.json (1 hunks)
  • packages/extension/public/locales/en/translation.json (1 hunks)
  • packages/extension/src/extension.ts (1 hunks)
  • packages/extension/src/packageInfo.ts (1 hunks)
  • packages/extension/webpack.shared.cjs (4 hunks)
✅ Files skipped from review due to trivial changes (1)
  • packages/extension/public/locales/en/translation.json
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/extension/src/packageInfo.ts
  • packages/extension/package.json
  • package.json
🔇 Additional comments (4)
packages/extension/webpack.shared.cjs (3)

13-13: LGTM: Sentry webpack plugin import

The import is correctly placed with other webpack plugins.


95-95: LGTM: Secure environment configuration

Good practice using environment variable with a secure default to 'production'.


32-32: ⚠️ Potential issue

Security: Protect source maps in production

While source maps are required for Sentry, exposing them in production could reveal sensitive source code.

Apply this change to secure source maps while maintaining Sentry functionality:

-  devtool: 'source-map', // Source map generation must be turned on
+  devtool: process.env.NODE_ENV === 'production' ? 'hidden-source-map' : 'source-map',

This ensures source maps are:

  • Available for Sentry error reporting
  • Hidden from public access in production
  • Accessible during development
packages/extension/src/extension.ts (1)

17-34: ⚠️ Potential issue

Security and performance concerns in Sentry configuration

Several critical issues need to be addressed:

  1. The DSN should be moved to environment variables
  2. Sampling rates are too high for production
  3. Extension IDs should be configurable

Additionally:

  • Consider adding environment-specific configuration
  • Add error boundaries around Sentry initialization
  • Add proper error handling for initialization failures

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

🧹 Outside diff range and nitpick comments (1)
.github/workflows/ci-workflow.yml (1)

80-80: Remove trailing whitespace

Lines 80 and 85 contain trailing spaces that should be removed.

Apply this diff to fix the formatting:

       - name: Build project
         run: yarn build
-  
+
       - name: Upload source maps to Sentry
         env:
           SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
         run: npx sentry-cli upload-sourcemaps ./packages/extension/build
-  
+
       - name: Run Semantic Release

Also applies to: 85-85

🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 664de2a and f3fbfc5.

📒 Files selected for processing (1)
  • .github/workflows/ci-workflow.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 80-80: trailing spaces

(trailing-spaces)


[error] 85-85: trailing spaces

(trailing-spaces)

🔇 Additional comments (2)
.github/workflows/ci-workflow.yml (2)

78-79: LGTM: Build step correctly positioned

The build step is appropriately placed before the source map upload step, ensuring that the build artifacts are available.


81-84: ⚠️ Potential issue

Add missing Sentry configuration

The Sentry upload step is missing required organization and project configuration.

Apply this diff to add the missing configuration:

       - name: Upload source maps to Sentry
         env:
           SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
+          SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
+          SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
         run: npx sentry-cli upload-sourcemaps ./packages/extension/build

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

🧹 Outside diff range and nitpick comments (1)
packages/extension/webpack.shared.cjs (1)

Line range hint 37-43: Consider enhancing CSS processing for production

While the current CSS configuration works, consider adding optimizations for production builds.

Consider this enhanced configuration:

  {
    test: /\.css$/,
-   use: ['style-loader', 'css-loader']
+   use: [
+     'style-loader',
+     {
+       loader: 'css-loader',
+       options: {
+         importLoaders: 1,
+         modules: {
+           auto: true,
+           localIdentName: process.env.NODE_ENV === 'production'
+             ? '[hash:base64]'
+             : '[path][name]__[local]'
+         }
+       }
+     },
+     'postcss-loader'
+   ]
  }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between f3fbfc5 and 44cc8df.

📒 Files selected for processing (1)
  • packages/extension/webpack.shared.cjs (4 hunks)
🔇 Additional comments (2)
packages/extension/webpack.shared.cjs (2)

32-32: ⚠️ Potential issue

Secure source map access for Sentry integration

While source maps are required for Sentry error tracking, exposing them in production could reveal sensitive source code.

Consider using a more secure configuration:

-  devtool: 'source-map', // Source map generation must be turned on
+  devtool: process.env.NODE_ENV === 'production' 
+    ? 'hidden-source-map'  // Generates source maps but doesn't reference them in bundles
+    : 'source-map',

110-120: Verify Sentry authentication configuration

The Sentry configuration looks good with appropriate privacy settings. However, let's ensure the auth token is properly configured.

✅ Verification successful

Let me gather more information about Sentry configuration in the repository.


Let me try one more verification with corrected commands to check workflow files and package.json content.


Let me check the CI workflow file to understand how the Sentry token is handled.


Sentry authentication is properly configured

The Sentry configuration is correctly set up:

  • The auth token is securely managed as a GitHub secret in CI workflow
  • Token is properly injected during the build process for source map uploads
  • Development builds skip Sentry integration as intended
  • Required Sentry dependencies are present in package.json
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Sentry auth token configuration in environment files

# Check if SENTRY_AUTH_TOKEN is defined in .env file
echo "Checking .env file for Sentry configuration..."
if [ -f ".env" ]; then
  grep -l "SENTRY_AUTH_TOKEN" .env || echo "Warning: SENTRY_AUTH_TOKEN not found in .env"
fi

# Check if there are any CI environment files
echo "Checking CI environment files..."
fd -t f ".env" .github/

Length of output: 219


Script:

#!/bin/bash
# Check for Sentry configuration in various potential locations
echo "Checking for Sentry configuration files..."
fd -t f "sentry" -E "node_modules"

echo -e "\nChecking for Sentry references in package.json files..."
fd -t f "package.json" -E "node_modules" -x grep -l "sentry"

echo -e "\nChecking for environment variable setup in GitHub workflows..."
fd -t f "*.yml" .github/workflows/ -x grep -l "SENTRY_AUTH_TOKEN"

echo -e "\nChecking for environment setup documentation..."
fd -t f "README.md" -x grep -A 3 -i "sentry"

Length of output: 972


Script:

#!/bin/bash
# Check package.json for Sentry dependencies and configurations
echo "Checking package.json content..."
cat package.json | grep -A 5 "sentry"

echo -e "\nChecking workflow files for Sentry token setup..."
fd -g "*.yml" .github/workflows/ -x grep -l "SENTRY_AUTH_TOKEN" {} \;

echo -e "\nChecking for environment variable documentation..."
rg -i "sentry.*token" -A 3 --type md

Length of output: 747


Script:

#!/bin/bash
# Check the CI workflow file for Sentry token configuration
echo "Checking CI workflow configuration..."
cat .github/workflows/ci-workflow.yml | grep -A 5 -B 5 "SENTRY_AUTH_TOKEN"

# Check if there's any documentation about environment setup
echo -e "\nChecking for environment setup instructions..."
rg -i "environment|env" README.md

Length of output: 645

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

🧹 Outside diff range and nitpick comments (2)
.github/workflows/ci-workflow.yml (2)

78-79: Consider optimizing by persisting build artifacts

While this build step is necessary for Sentry source maps upload, you could optimize the workflow by persisting the build artifacts from the earlier build job using GitHub Actions artifacts.

Here's how you could modify the workflow:

  build:
    steps:
      # ... existing steps ...
      - name: ${{ matrix.step }}
        run: yarn ${{ matrix.step }}
+     - name: Upload build artifacts
+       uses: actions/upload-artifact@v3
+       with:
+         name: build-artifacts
+         path: packages/extension/build

  release:
    steps:
      # ... existing steps ...
+     - name: Download build artifacts
+       uses: actions/download-artifact@v3
+       with:
+         name: build-artifacts
+         path: packages/extension/build
-     - name: Build project
-       run: yarn build

80-80: Remove trailing spaces

There are trailing spaces on lines 80 and 85.

Apply this diff to fix the formatting:

      - name: Build project
        run: yarn build
-  
+
      - name: Upload source maps to Sentry
        env:
          SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
        run: npx sentry-cli sourcemaps upload ./packages/extension/build
-  
+
      - name: Run Semantic Release

Also applies to: 85-85

🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 44cc8df and 727d86b.

📒 Files selected for processing (1)
  • .github/workflows/ci-workflow.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 80-80: trailing spaces

(trailing-spaces)


[error] 85-85: trailing spaces

(trailing-spaces)

🔇 Additional comments (1)
.github/workflows/ci-workflow.yml (1)

81-84: ⚠️ Potential issue

Complete the Sentry configuration

The source map upload step is missing required Sentry configuration parameters.

Apply this diff to add the missing configuration:

  - name: Upload source maps to Sentry
    env:
      SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
+     SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
+     SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
    run: npx sentry-cli sourcemaps upload ./packages/extension/build

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: 3

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 727d86b and 26b5bf2.

📒 Files selected for processing (1)
  • .github/workflows/ci-workflow.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-workflow.yml

79-79: shellcheck reported issue in this script: SC2086:info:1:62: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 83-83: trailing spaces

(trailing-spaces)


[error] 86-86: trailing spaces

(trailing-spaces)

🔇 Additional comments (1)
.github/workflows/ci-workflow.yml (1)

78-86: Verify end-to-end release process

Please ensure that the entire release process works correctly with these new Sentry integration steps:

  1. Sentry authentication and configuration
  2. Production build generation
  3. Source map upload
  4. Semantic release
✅ Verification successful

Let me gather more information about the Sentry configuration and release process.


Based on the gathered information, I can now provide the final response:

Sentry integration appears properly configured for the release process

The verification shows that:

  • Sentry is properly initialized in packages/extension/src/extension.ts with DSN and browser tracing
  • Source maps are correctly configured in webpack.shared.cjs with devtool: 'source-map'
  • The build process is well-structured with proper sequencing:
    1. Version application (apply-version script)
    2. Production build (build script)
    3. Source map generation (webpack config)
    4. Release process (semantic-release)

The integration steps in the workflow align with the project's configuration and should work correctly end-to-end.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify Sentry configuration and build process

# Check if Sentry CLI is configured in the project
echo "Checking for Sentry configuration..."
fd -t f -e json -e js -e yml "sentry" 

# Check build output structure
echo "Checking build configuration..."
fd -t f "webpack|build" -e js -e cjs -e json

# Check for source map configuration
echo "Checking source map configuration..."
rg -i "sourcemap|devtool" 

Length of output: 1809


Script:

#!/bin/bash
# Check for Sentry configuration and release process setup

# Look for Sentry initialization and configuration
echo "Checking Sentry initialization..."
rg -i "sentry.init|sentry.config" -A 3

# Check for release-related configuration
echo "Checking release configuration..."
rg -i "release.*version|sentry.*release" -A 2

# Check package.json for build and release scripts
echo "Checking build and release scripts..."
fd -t f "package.json" --exec cat {} \; | jq -r '.scripts | select(. != null)'

# Check for semantic release configuration
echo "Checking semantic release config..."
fd -t f -e json -e yml -e yaml "release"

Length of output: 2811

🧰 Tools
🪛 actionlint (1.7.4)

79-79: shellcheck reported issue in this script: SC2086:info:1:62: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)

[error] 83-83: trailing spaces

(trailing-spaces)


[error] 86-86: trailing spaces

(trailing-spaces)

.github/workflows/ci-workflow.yml Outdated Show resolved Hide resolved
.github/workflows/ci-workflow.yml Outdated Show resolved Hide resolved
.github/workflows/ci-workflow.yml Outdated Show resolved Hide resolved
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: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 26b5bf2 and b9443f3.

📒 Files selected for processing (2)
  • .github/workflows/ci-workflow.yml (1 hunks)
  • packages/extension/webpack.shared.cjs (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/extension/webpack.shared.cjs
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/ci-workflow.yml

86-86: shellcheck reported issue in this script: SC2086:info:1:67: Double quote to prevent globbing and word splitting

(shellcheck)


86-86: shellcheck reported issue in this script: SC2086:info:1:89: Double quote to prevent globbing and word splitting

(shellcheck)

🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 80-80: trailing spaces

(trailing-spaces)


[error] 87-87: trailing spaces

(trailing-spaces)

🔇 Additional comments (2)
.github/workflows/ci-workflow.yml (2)

78-80: Enhance build step configuration

The build step should be enhanced with production environment setting and build verification.

Apply this diff to improve the build step:

       - name: Build project
-        run: yarn build
+        env:
+          NODE_ENV: production
+        run: |
+          yarn build
+          test -d ./packages/extension/build || exit 1
🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)


81-86: Enhance source map upload configuration

The source map upload step should include version tracking and validation.

Apply this diff to improve the source map upload:

       - name: Upload source maps to Sentry
+        run: |
+          # Verify source maps exist
+          if ! find ./packages/extension/build -name "*.map" -type f | grep -q .; then
+            echo "No source maps found in build directory"
+            exit 1
+          fi
+          
+          # Upload source maps with version tracking
+          VERSION=$(node -p "require('./package.json').version")
+          npx sentry-cli releases new "$VERSION"
+          npx sentry-cli releases files "$VERSION" upload-sourcemaps ./packages/extension/build --org "$SENTRY_ORG" --project "$SENTRY_PROJECT"
+          npx sentry-cli releases finalize "$VERSION"
         env:
           SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
           SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
           SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
-        run: npx sentry-cli sourcemaps upload ./packages/extension/build --org $SENTRY_ORG --project $SENTRY_PROJECT
🧰 Tools
🪛 actionlint (1.7.4)

86-86: shellcheck reported issue in this script: SC2086:info:1:67: Double quote to prevent globbing and word splitting

(shellcheck)


86-86: shellcheck reported issue in this script: SC2086:info:1:89: Double quote to prevent globbing and word splitting

(shellcheck)

.github/workflows/ci-workflow.yml Outdated Show resolved Hide resolved
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

🧹 Outside diff range and nitpick comments (2)
.github/workflows/ci-workflow.yml (2)

80-80: Remove trailing whitespace

There is a trailing whitespace on line 80 that should be removed.

-        run: yarn build
-  
+        run: yarn build
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)


78-80: Add Sentry source map upload step

Since this PR introduces Sentry integration, we should add a step to upload source maps to Sentry after the build.

Add this step after the build:

      - name: Upload source maps to Sentry
        env:
          SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
          SENTRY_ORG: ${{ secrets.SENTRY_ORG }}
          SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
        run: |
          VERSION=$(node -p "require('./package.json').version")
          npx sentry-cli releases new "$VERSION"
          npx sentry-cli releases files "$VERSION" upload-sourcemaps ./packages/extension/build
          npx sentry-cli releases finalize "$VERSION"

Please ensure the following secrets are configured in your repository:

  • SENTRY_AUTH_TOKEN
  • SENTRY_ORG
  • SENTRY_PROJECT
🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b9443f3 and b16cdaa.

📒 Files selected for processing (1)
  • .github/workflows/ci-workflow.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 80-80: trailing spaces

(trailing-spaces)

🔇 Additional comments (2)
.github/workflows/ci-workflow.yml (2)

78-80: Verify necessity of additional build step

The release job already depends on the build job (see needs: [build]). This additional build step might be redundant unless there's a specific reason for rebuilding before release.

Let's verify the build artifacts handling:

Consider either:

  1. Removing this build step if it's redundant, or
  2. Adding artifact upload/download steps to preserve the build output between jobs
✅ Verification successful

Let me gather more information about the workflow dependencies and build job configuration.

This will help us understand:

  1. The exact relationship between the build and release jobs
  2. Whether the build output needs to be preserved between jobs
  3. The configuration of build artifacts in the project

Build step is necessary in release job

The additional build step in the release job is necessary and not redundant. Here's why:

  • The build job and release job run as separate jobs with separate workspaces
  • No artifact upload/download steps exist between jobs
  • The build process creates zip files (master-build.zip, master-src.zip) that are likely needed for the release
  • The semantic-release process would need access to the built artifacts to create the release
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if build artifacts are preserved between jobs

# Check for artifact upload/download steps
echo "Checking for artifact handling between jobs:"
rg "actions/upload-artifact|actions/download-artifact" .github/workflows/

# Check if build output is needed for release
echo "Checking release script requirements:"
rg "semantic-release" package.json -A 5

Length of output: 1235


Script:

#!/bin/bash
# Check the build job configuration and job dependencies
echo "Checking workflow configuration:"
cat .github/workflows/ci-workflow.yml

# Check if there are any build artifacts or dist files that need to be preserved
echo -e "\nChecking build output paths:"
rg "dist|build" package.json -A 2

Length of output: 3371

🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)


78-80: 🛠️ Refactor suggestion

Enhance build step configuration for production deployment

The build step should be configured for production and include verification of the build output. Additionally, since this PR introduces Sentry integration, we should ensure source maps are generated correctly.

Apply this diff to improve the build step:

       - name: Build project
+        env:
+          NODE_ENV: production
+          GENERATE_SOURCEMAP: true
         run: |
-          yarn build
+          yarn build
+          # Verify build output
+          test -d ./packages/extension/build || exit 1
+          # Verify source maps for Sentry
+          find ./packages/extension/build -name "*.map" -type f || echo "Warning: No source maps found"

Likely invalid or redundant comment.

🧰 Tools
🪛 yamllint (1.35.1)

[error] 80-80: trailing spaces

(trailing-spaces)

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

🧹 Outside diff range and nitpick comments (2)
.github/workflows/ci-workflow.yml (2)

Line range hint 78-81: Enhance semantic-release configuration

The semantic-release setup needs additional configuration:

  1. Consider using the conventional GITHUB_TOKEN name instead of SR_TOKEN for better maintainability
  2. Add semantic-release configuration to specify release rules and plugins

Apply this diff to enhance the configuration:

       - name: Run Semantic Release
         env:
-          GH_TOKEN: ${{ secrets.SR_TOKEN }}
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
         run: npx semantic-release

Also, create a .releaserc.json file in the root directory:

{
  "branches": ["main"],
  "plugins": [
    "@semantic-release/commit-analyzer",
    "@semantic-release/release-notes-generator",
    "@semantic-release/github"
  ]
}
🧰 Tools
🪛 yamllint (1.35.1)

[error] 77-77: trailing spaces

(trailing-spaces)


77-77: Remove trailing spaces

There are trailing spaces on line 77 that should be removed for consistency.

Apply this diff to fix the formatting:

-  
+
🧰 Tools
🪛 yamllint (1.35.1)

[error] 77-77: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between b16cdaa and 6a46490.

📒 Files selected for processing (1)
  • .github/workflows/ci-workflow.yml (1 hunks)
🧰 Additional context used
🪛 yamllint (1.35.1)
.github/workflows/ci-workflow.yml

[error] 77-77: trailing spaces

(trailing-spaces)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants