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

Automated spellchecking with cspell [SLT-408] #3350

Merged
merged 7 commits into from
Oct 29, 2024
Merged

Conversation

abtestingalpha
Copy link
Collaborator

@abtestingalpha abtestingalpha commented Oct 28, 2024

Description
Adds cspell for automated spellchecking during CI. Package scripts directly affect:

  • docs
  • synapse-interface
  • explorer-ui

9628884: docs preview link
9628884: explorer-ui preview link

Summary by CodeRabbit

  • New Features

    • Added spellchecking capabilities to the linting process across all packages.
    • Introduced a new configuration file for spellchecking, enhancing documentation accuracy.
  • Bug Fixes

    • Corrected typographical errors in various documentation files and user interface messages, improving clarity and professionalism.
  • Documentation

    • Updated and clarified documentation for the Synapse Bridge SDK and REST API, including installation instructions and parameter descriptions.
  • Chores

    • Added new scripts for spellchecking in several package configurations to maintain spelling accuracy in the codebase.

cba005d: docs preview link
cba005d: explorer-ui preview link
cba005d: synapse-interface preview link
cdd17d5: docs preview link
cdd17d5: explorer-ui preview link
cdd17d5: synapse-interface preview link
e4e8c71: docs preview link
e4e8c71: explorer-ui preview link
e4e8c71: synapse-interface preview link
30f978c: docs preview link
30f978c: explorer-ui preview link
30f978c: synapse-interface preview link
d2acf5d: docs preview link
d2acf5d: explorer-ui preview link
d2acf5d: synapse-interface preview link

Copy link
Contributor

coderabbitai bot commented Oct 28, 2024

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

📥 Commits

Files that changed from the base of the PR and between 421a343 and 0385800.

📒 Files selected for processing (3)
  • docs/bridge/package.json (2 hunks)
  • packages/explorer-ui/package.json (2 hunks)
  • packages/synapse-interface/package.json (2 hunks)
_____________________________________________________________________________________________________________________
< The average user doesn't give a damn what happens, as long as (1) it works and (2) it's fast. - Daniel J. Bernstein >
---------------------------------------------------------------------------------------------------------------------
 \
  \   \
       \ /\
       ( )
     .( o ).

Walkthrough

The pull request introduces enhancements to the TypeScript workflow by adding a spellcheck step in the linting process and updating the test job for better file change tracking. A new configuration file, cspell.json, is added to specify spell-checking rules and custom dictionaries. Documentation files across various components are updated for typographical corrections and clarity, particularly in the Synapse Bridge SDK and REST API. Additionally, several localization files are modified to correct spelling errors in user messages.

Changes

File Change Summary
.github/workflows/packages.yml Added spellcheck step to lint job and improved file change tracking in test job.
cspell.json New configuration file for spell checking with custom words and ignore rules.
docs/bridge/README.md Corrected typo "regenration" to "regeneration".
docs/bridge/docs/02-Bridge/01-SDK.md Fixed typo "wiht" to "with", clarified options object structure in functions.
docs/bridge/docs/02-Bridge/02-REST-API.md Corrected "hexidecimal" to "hexadecimal" and "addres" to "address".
docs/bridge/docs/04-Routers/01-Synapse-Router/index.md Corrected typographical errors and clarified documentation.
docs/bridge/docs/04-Routers/RFQ/01-Relayer.md Updated terminology and enhanced explanations of relayer functionality.
docs/bridge/docs/04-Routers/RFQ/API/index.md Fixed spelling errors and minor formatting adjustments.
docs/bridge/docs/06-Services/04-Submitter.md Corrected "greatrer" to "greater" and "simultanously" to "simultaneously".
docs/bridge/package.json Added new spellcheck scripts and updated ci:spellcheck.
packages/explorer-ui/components/misc/CopyTitle.tsx Corrected "opaciyt" to "opacity" in class assignment.
packages/explorer-ui/package.json Added spellcheck scripts and new dependency for spell checking.
packages/sdk-router/package.json Added spellcheck scripts for README and new dependency for spell checking.
packages/synapse-constants/package.json Added new script and cleared devDependencies.
packages/synapse-interface/components/Maintenance/components/AnnouncementBanner.tsx Corrected local storage key typo.
packages/synapse-interface/components/Maintenance/example/EcotoneForkUpgrade.tsx Fixed typo in comment section.
packages/synapse-interface/constants/urls/uriToHttp.tsx Corrected comment from "udpate" to "update".
packages/synapse-interface/messages/*.json Corrected "Succesfully" to "Successfully" in multiple localization files.
packages/synapse-interface/package.json Added spellcheck scripts and new dependency for spell checking.
packages/synapse-interface/pages/state-managed-bridge/index.tsx Corrected success message and improved error handling logic.

Possibly related PRs

Suggested labels

size/s

Suggested reviewers

  • bigboydiamonds
  • abtestingalpha

Poem

In the land of code where bunnies hop,
We fixed some typos, made errors stop.
With spellcheck scripts, our docs shine bright,
Clarity and accuracy, oh what a sight!
Hopping through changes, we cheer and play,
For a cleaner codebase, hip-hip-hooray! 🐇✨


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.

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.

@abtestingalpha abtestingalpha changed the title Automated spellchecking with cspell Automated spellchecking with cspell [SLT-408] Oct 28, 2024
Copy link

cloudflare-workers-and-pages bot commented Oct 28, 2024

Deploying sanguine-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0385800
Status: ✅  Deploy successful!
Preview URL: https://5f81b936.sanguine-fe.pages.dev
Branch Preview URL: https://docs-spellcheck.sanguine-fe.pages.dev

View logs

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

🧹 Outside diff range and nitpick comments (11)
packages/solidity-devops/package.json (1)

26-26: Consider documenting the empty script's purpose.

The empty script is correctly added as part of the spellchecking integration. However, to improve maintainability, consider adding a comment explaining why this script is intentionally empty (e.g., "// No spellchecking needed for this package - executed at workspace level").

-    "ci:spellcheck": " ",
+    "ci:spellcheck": " ", // No spellchecking needed - executed at workspace level
packages/synapse-constants/package.json (1)

Line range hint 36-48: Move build tools to devDependencies

The following packages are build tools and should be in devDependencies:

  • @rollup/plugin-*
  • rollup
  • rollup-plugin-typescript2
  • typescript
  • babel-plugin-transform-export-extensions

Apply this diff:

  "dependencies": {
    "@ethersproject/address": "5.7.0",
    "@ethersproject/bignumber": "5.7.0",
-   "@rollup/plugin-commonjs": "^28.0.0",
-   "@rollup/plugin-json": "^6.1.0",
-   "@rollup/plugin-node-resolve": "^15.3.0",
-   "@rollup/plugin-terser": "^0.4.4",
-   "@rollup/plugin-url": "^8.0.2",
-   "babel-plugin-transform-export-extensions": "^6.22.0",
    "ethers": "5.7.2",
    "lodash": "^4.17.21",
-   "rollup": "^4.22.4",
-   "rollup-plugin-typescript2": "^0.36.0",
-   "typescript": "^5.3.3",
  },
  "devDependencies": {
+   "@rollup/plugin-commonjs": "^28.0.0",
+   "@rollup/plugin-json": "^6.1.0",
+   "@rollup/plugin-node-resolve": "^15.3.0",
+   "@rollup/plugin-terser": "^0.4.4",
+   "@rollup/plugin-url": "^8.0.2",
+   "babel-plugin-transform-export-extensions": "^6.22.0",
+   "rollup": "^4.22.4",
+   "rollup-plugin-typescript2": "^0.36.0",
+   "typescript": "^5.3.3"
  }
docs/bridge/package.json (1)

Inconsistent spellcheck implementation across packages needs standardization

The verification reveals inconsistencies in spellcheck implementation:

  • Only 4 packages implement ci:spellcheck with varying patterns:
    • bridge-docs: checks **/*.{md,mdx}
    • explorer-ui and synapse-interface: check **/*.{tsx,jsx,md,mdx}
    • sdk-router: only checks README.md
  • A root cspell.json exists with comprehensive configuration including custom words, dictionaries, and ignore patterns
  • Other packages lack spellcheck scripts despite having documentation

Recommendations:

  • Standardize ci:spellcheck script across all packages with documentation
  • Use consistent glob patterns that match the package's content type
  • Leverage the existing root cspell.json configuration
🔗 Analysis chain

Line range hint 18-51: Verify consistent spellchecking setup across packages.

Let's ensure the spellchecking configuration is consistent across all packages mentioned in the summary.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for cspell.json and consistent script implementation

# Look for cspell.json files
echo "Checking for cspell configuration files:"
fd "cspell.json"

# Check for ci:spellcheck script in package.json files
echo -e "\nChecking for consistent spellcheck script implementation:"
fd "package.json" | xargs jq -r 'select(.scripts."ci:spellcheck" != null) | "\(.name): \(.scripts."ci:spellcheck")"'

Length of output: 942


Script:

#!/bin/bash
# Let's check the content of cspell.json to understand the global configuration
echo "Content of root cspell.json:"
cat cspell.json

# Let's also check if there are any package-specific cspell configurations
echo -e "\nPackage-specific cspell configurations:"
fd -e json -e jsonc -e yaml -e yml | grep -i "cspell\|spell"

Length of output: 2916

packages/explorer-ui/package.json (1)

76-76: Maintain alphabetical ordering in devDependencies.

The cspell package is incorrectly positioned. It should be placed after the @storybook dependencies to maintain alphabetical ordering.

-    "cspell": "^8.15.4",
     "@codecov/webpack-plugin": "^0.0.1-beta.10",
     "@storybook/addon-actions": "^6.5.15",
     "@storybook/addon-essentials": "^6.5.15",
     "@storybook/addon-interactions": "^6.5.15",
     "@storybook/addon-links": "^6.5.15",
     "@storybook/builder-webpack5": "^6.5.15",
     "@storybook/manager-webpack5": "^6.5.15",
     "@storybook/react": "^6.5.15",
     "@storybook/testing-library": "^0.0.13",
     "autoprefixer": "^10.4.7",
     "babel-loader": "^8.3.0",
+    "cspell": "^8.15.4",
     "postcss": "^8.4.13",
packages/synapse-interface/components/Maintenance/components/AnnouncementBanner.tsx (1)

Line range hint 34-42: Consider adding a migration for existing users.

To prevent the banner from reappearing for users who previously dismissed it, consider adding a migration that checks for the old key:

 useEffect(() => {
   if (hasMounted && isStarted && !isComplete) {
+    // Migrate from old misspelled key
+    const oldStoredValue = localStorage.getItem('showAnnoucementBanner')
+    if (oldStoredValue !== null) {
+      localStorage.setItem('showAnnouncementBanner', oldStoredValue)
+      localStorage.removeItem('showAnnoucementBanner')
+    }
+
     const storedShowBanner = localStorage.getItem('showAnnouncementBanner')
     const storedBannerId = localStorage.getItem('bannerId')
.github/workflows/packages.yml (1)

48-51: Consider adding job-level documentation

The test job would benefit from a comment explaining its purpose and the significance of the SDK_RPC_URL environment variable.

  test:
+   # Runs unit tests for all packages and reports coverage to Codecov
+   # Requires SDK_RPC_URL for integration tests with external services
    runs-on: ubuntu-latest
    env:
      SDK_RPC_URL: ${{ secrets.SDK_RPC_URL }}
packages/synapse-interface/package.json (1)

25-25: Consider extending spellcheck coverage.

The script looks good but could be enhanced to include more file types commonly containing text that needs spellchecking.

Consider updating the script to include more file extensions:

-    "ci:spellcheck": "cspell \"**/*.{tsx,jsx,md,mdx}\" --show-suggestions",
+    "ci:spellcheck": "cspell \"**/*.{ts,tsx,js,jsx,md,mdx}\" --show-suggestions --config cspell.json",
docs/bridge/docs/02-Bridge/01-SDK.md (2)

Line range hint 1-400: Consider enhancing the documentation structure and clarity.

While the documentation is comprehensive, consider these improvements for better readability:

  1. Add a table of contents at the beginning for easier navigation
  2. Include a "Quick Start" section with a minimal working example
  3. Add version compatibility information for the SDK
  4. Include troubleshooting section for common issues

Would you like me to help draft these sections?

🧰 Tools
🪛 Markdownlint

39-39: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)


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

Documentation needs updates for SDK version 0.11.4

The code examples in the documentation are outdated and need updates:

  • Documentation still references version 0.10.0 breaking changes while the current SDK version is 0.11.4
  • The bridgeQuote function signature in the actual implementation shows it accepts an options parameter of type BridgeQuoteOptions, but the documentation doesn't fully reflect this structure
  • The Node.js version requirement in the documentation (v16+) is outdated as package.json specifies "node": ">=18.17.0"
🔗 Analysis chain

Line range hint 1-400: Verify code examples are up to date.

The code examples should be verified to ensure they match the latest SDK version, especially given the breaking changes in version 0.10.0.


Let me analyze the code examples in the documentation against the actual SDK implementation to verify if they are up to date.


Let me check the actual implementation of bridgeQuote function with a different pattern.


Let me try one final search to verify the SDK implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for SDK version references in the codebase
rg -A 2 "SynapseSDK" 
# Search for example usages that might need updates
rg "bridgeQuote\(" 

Length of output: 21446


Script:

#!/bin/bash
# Check SDK version in package.json
fd -g "package.json" packages/sdk-router/ -x cat {} 

# Get actual implementation of bridgeQuote
ast-grep --pattern 'export async function bridgeQuote($$$)' 

Length of output: 2639


Script:

#!/bin/bash
# Get the bridgeQuote function implementation
ast-grep --pattern 'bridgeQuote($$$) {
  $$$
}'

# Check for any breaking changes documentation
rg -A 5 "breaking changes" packages/sdk-router/

Length of output: 99


Script:

#!/bin/bash
# Search for bridgeQuote function definition
rg -A 10 "export (async )?function bridgeQuote" packages/sdk-router/src/

# Search for the latest version changes
rg -A 10 "Version 0\.1[0-9]" packages/sdk-router/

Length of output: 938

🧰 Tools
🪛 Markdownlint

39-39: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)

docs/bridge/docs/04-Routers/01-Synapse-Router/index.md (1)

Line range hint 279-282: Consider adding a note about allowance security best practices

While the allowance check is correct, it might be worth adding a note about the security implications of setting unlimited allowances. Consider mentioning that users should prefer setting exact allowances rather than maximum values.

Add a comment before the allowance check:

    // If user selected an ERC-20 token as tokenIn, we would need to use msg.value=0
    amountETH = 0
+   // Note: For security, prefer setting exact allowances over maximum values
    // We also need to check if user approved routerOrigin to spend `tokenIn`
docs/bridge/docs/04-Routers/RFQ/01-Relayer.md (1)

Line range hint 4-4: Consider tracking TODO comments as GitHub issues.

There are several TODO comments throughout the documentation that should be tracked:

  1. Time parameter for quotes (around line 40)
  2. Auto-generation of configuration documentation (around line 90)
  3. Additional information about relayer API port security (around line 95)

Would you like me to create GitHub issues to track these TODOs?

🧰 Tools
🪛 Markdownlint

63-63: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time

(MD001, heading-increment)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e590bd9 and bafc943.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (35)
  • .github/workflows/packages.yml (1 hunks)
  • cspell.json (1 hunks)
  • docs/bridge/README.md (1 hunks)
  • docs/bridge/docs/02-Bridge/01-SDK.md (1 hunks)
  • docs/bridge/docs/02-Bridge/02-REST-API.md (3 hunks)
  • docs/bridge/docs/04-Routers/01-Synapse-Router/index.md (6 hunks)
  • docs/bridge/docs/04-Routers/RFQ/01-Relayer.md (2 hunks)
  • docs/bridge/docs/04-Routers/RFQ/API/index.md (4 hunks)
  • docs/bridge/docs/06-Services/04-Submitter.md (2 hunks)
  • docs/bridge/package.json (2 hunks)
  • packages/contracts-core/package.json (1 hunks)
  • packages/contracts-rfq/package.json (1 hunks)
  • packages/coverage-aggregator/package.json (1 hunks)
  • packages/explorer-ui/components/misc/CopyTitle.tsx (1 hunks)
  • packages/explorer-ui/package.json (2 hunks)
  • packages/rest-api/package.json (1 hunks)
  • packages/rfq-indexer/api/package.json (1 hunks)
  • packages/rfq-indexer/indexer/package.json (1 hunks)
  • packages/rfq-loadtest/package.json (1 hunks)
  • packages/sdk-router/package.json (2 hunks)
  • packages/solidity-devops/package.json (1 hunks)
  • packages/synapse-constants/package.json (2 hunks)
  • packages/synapse-interface/components/Maintenance/components/AnnouncementBanner.tsx (2 hunks)
  • packages/synapse-interface/components/Maintenance/example/EcotoneForkUpgrade.tsx (1 hunks)
  • packages/synapse-interface/constants/urls/uriToHttp.tsx (1 hunks)
  • packages/synapse-interface/messages/ar.json (1 hunks)
  • packages/synapse-interface/messages/en-US.json (1 hunks)
  • packages/synapse-interface/messages/es.json (1 hunks)
  • packages/synapse-interface/messages/fr.json (1 hunks)
  • packages/synapse-interface/messages/jp.json (1 hunks)
  • packages/synapse-interface/messages/tr.json (1 hunks)
  • packages/synapse-interface/messages/zh-CN.json (1 hunks)
  • packages/synapse-interface/package.json (2 hunks)
  • packages/synapse-interface/pages/state-managed-bridge/index.tsx (1 hunks)
  • packages/widget/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (15)
  • cspell.json
  • docs/bridge/README.md
  • packages/coverage-aggregator/package.json
  • packages/explorer-ui/components/misc/CopyTitle.tsx
  • packages/rfq-indexer/indexer/package.json
  • packages/rfq-loadtest/package.json
  • packages/synapse-interface/components/Maintenance/example/EcotoneForkUpgrade.tsx
  • packages/synapse-interface/constants/urls/uriToHttp.tsx
  • packages/synapse-interface/messages/ar.json
  • packages/synapse-interface/messages/en-US.json
  • packages/synapse-interface/messages/es.json
  • packages/synapse-interface/messages/fr.json
  • packages/synapse-interface/messages/jp.json
  • packages/synapse-interface/messages/zh-CN.json
  • packages/widget/package.json
🧰 Additional context used
🪛 actionlint
.github/workflows/packages.yml

88-88: property "verify-changed-files" is not defined in object type {verify-yarn-lock: {conclusion: string; outcome: string; outputs: {string => string}}}

(expression)


89-89: property "verify-changed-files" is not defined in object type {verify-yarn-lock: {conclusion: string; outcome: string; outputs: {string => string}}}

(expression)

🪛 LanguageTool
docs/bridge/docs/04-Routers/RFQ/API/index.md

[uncategorized] ~16-~16: Possible missing comma found.
Context: ...hecking on-chain balances and in-flight requests and other implementations should take a...

(AI_HYDRA_LEO_MISSING_COMMA)


[typographical] ~73-~73: It seems that a comma is missing.
Context: ...lite, this can be a path, if using mysql please see [here](https://dev.mysql.com/doc/co...

(IF_PLEASE_COMMA)

docs/bridge/docs/06-Services/04-Submitter.md

[grammar] ~85-~85: Using ‘couple’ without ‘of’ is considered to be informal.
Context: ...happen simultaneously. You will need a couple things in order to use the Transaction submitt...

(PLENTY_OF_NOUNS)


[style] ~85-~85: Consider a shorter alternative to avoid wordiness.
Context: ...neously. You will need a couple things in order to use the Transaction submitter that sign...

(IN_ORDER_TO_PREMIUM)

🪛 Markdownlint
docs/bridge/docs/04-Routers/RFQ/API/index.md

54-54: Punctuation: ':'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)

🔇 Additional comments (13)
packages/sdk-router/package.json (1)

60-61: LGTM! Proper dependency configuration

The cspell dependency is correctly added as a dev dependency with an appropriate version constraint, and the JSON formatting is maintained with proper comma placement.

.github/workflows/packages.yml (1)

44-45: LGTM: Spellcheck integration looks good!

The parallel execution using lerna is an efficient approach for running spellcheck across all packages.

packages/synapse-interface/package.json (1)

104-104: LGTM! Dependency added correctly.

The cspell package is correctly added as a development dependency with an appropriate version constraint.

docs/bridge/docs/02-Bridge/01-SDK.md (1)

37-37: LGTM! Clear and concise introduction to provider setup.

The added line provides a good transition into the provider setup section, making the documentation flow more logical.

docs/bridge/docs/02-Bridge/02-REST-API.md (3)

15-17: LGTM! Table formatting is clean and consistent.

The use of non-breaking hyphens in the date ensures proper display across different viewports.


41-41: LGTM! Spelling correction improves documentation accuracy.

The term "hexadecimal" is now correctly spelled.


199-199: LGTM! Parameter name is now correctly spelled.

The "address" parameter is properly documented.

docs/bridge/docs/04-Routers/01-Synapse-Router/index.md (2)

28-28: LGTM: Correct spelling of "bridgeable"

The spelling correction from "bridgable" to "bridgeable" is accurate and improves documentation quality.


59-61: LGTM: Clear and correct code examples

The code examples effectively demonstrate request mapping and query selection with proper syntax and consistent formatting.

Also applies to: 73-75

docs/bridge/docs/04-Routers/RFQ/01-Relayer.md (2)

65-65: LGTM: Spelling correction in the table header.

The spelling correction from "maintenence_pct" to "maintenance_pct" improves documentation accuracy and consistency.


175-175: LGTM: Improved CCTP address description.

The description has been corrected from "ccctp contract address" to "cctp contract address", making it more accurate.

packages/synapse-interface/pages/state-managed-bridge/index.tsx (1)

386-386: LGTM! Spelling correction looks good.

The spelling correction from "Succesfully" to "Successfully" aligns with proper English spelling.

Let's verify if this string needs to be updated in the translation files:

✅ Verification successful

No translation updates needed - all files are already using correct spelling

The spelling correction in the code is already reflected correctly in all translation files. The message "Successfully initiated bridge..." is properly spelled in all language files (ar.json, en-US.json, jp.json, es.json, zh-CN.json, tr.json, and fr.json) with no instances of the misspelled "Succesfully" found.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the success message needs to be updated in translation files
# Look for any instances of "Succesfully" in translation files
rg -i "succesfully" "public/locales"

Length of output: 89


Script:

#!/bin/bash
# Let's first find where translation files might be located
fd -t d "locales\|translations\|i18n"

# Then search for any instances of "Succesfully" in the entire codebase
# to ensure we don't miss any translation files
rg -i "succesfully" 

Length of output: 1867


Script:

#!/bin/bash
# Let's check for the specific success message pattern in the codebase
rg -i "Successfully initiated bridge from.*on.*to.*on" -A 1 -B 1

Length of output: 3341

packages/synapse-interface/messages/tr.json (1)

40-40: LGTM! Spelling correction aligns with PR objectives.

The spelling correction from "Succesfully" to "Successfully" in the template string is appropriate and consistent with the PR's goal of implementing automated spellchecking.

Let's verify this correction across other language files:

✅ Verification successful

Spelling correction is consistent across all language files

The verification confirms that:

  • No instances of the misspelled word "Succesfully" exist in any language file
  • The correct spelling "Successfully" is consistently used across all localization files (ar.json, en-US.json, es.json, fr.json, jp.json, tr.json, zh-CN.json)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the spelling correction across all language files
# Expected: The word "Successfully" should be consistently spelled in all files

# Search for any remaining instances of the misspelling
rg -i "succesfully" packages/synapse-interface/messages/

# Verify the correction in other language files
rg -i "successfully.*initiated.*bridge" packages/synapse-interface/messages/

Length of output: 1949

packages/contracts-rfq/package.json Outdated Show resolved Hide resolved
packages/rest-api/package.json Outdated Show resolved Hide resolved
packages/contracts-core/package.json Outdated Show resolved Hide resolved
packages/contracts-core/package.json Outdated Show resolved Hide resolved
packages/synapse-constants/package.json Outdated Show resolved Hide resolved
packages/explorer-ui/package.json Outdated Show resolved Hide resolved
.github/workflows/packages.yml Outdated Show resolved Hide resolved
.github/workflows/packages.yml Outdated Show resolved Hide resolved
.github/workflows/packages.yml Outdated Show resolved Hide resolved
Copy link

codecov bot commented Oct 28, 2024

Bundle Report

Changes will decrease total bundle size by 3.44MB (-9.64%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
sdk-router-@synapsecns/sdk-router-esm* 255.26kB 609 bytes (0.24%) ⬆️
sdk-router-@synapsecns/sdk-router-cjs* 117.55kB 312 bytes (0.27%) ⬆️
explorer-ui-server-cjs* 724.87kB 141.19kB (-16.3%) ⬇️
explorer-ui-client-array-push* 2.17MB 146.28kB (-6.32%) ⬇️
synapse-interface-client-array-push* 7.49MB 216.91kB (2.98%) ⬆️
synapse-interface-server-cjs* 1.65MB 156.93kB (10.5%) ⬆️
widget-cjs-esm* 271.27kB 2.03kB (-0.74%) ⬇️
docs-bridge-client-array-push 7.34MB 564.45kB (-7.14%) ⬇️
docs-bridge-server-cjs 11.57MB 3.31MB (-22.23%) ⬇️
synapse-constants-esm-cjs* 174.63kB 174.63kB (100%) ⬆️
synapse-constants-cjs-esm* 174.18kB 174.18kB (100%) ⬆️

ℹ️ *Bundle size includes cached data from a previous commit

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

🧹 Outside diff range and nitpick comments (1)
packages/synapse-interface/components/Activity/Transaction/TransactionOptions.tsx (1)

Line range hint 180-186: Improve type safety of spread props.

The component accepts untyped props through ...props: any, which could lead to runtime errors if invalid props are passed.

Consider explicitly typing the allowed props:

export function DropdownButton({
  open,
  onClick,
  isDelayed,
  className,
  ...props
}: {
  open: boolean
  onClick?: () => void
  isDelayed?: boolean
  className?: string
- props?: any
+ } & Omit<React.ButtonHTMLAttributes<HTMLButtonElement>, 'onClick' | 'className'>) {
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between bafc943 and 55bedbb.

📒 Files selected for processing (3)
  • packages/synapse-interface/components/Activity/Transaction/TransactionOptions.tsx (1 hunks)
  • packages/synapse-interface/components/Pools/PoolActionOptions.tsx (1 hunks)
  • packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx (2 hunks)
🔇 Additional comments (4)
packages/synapse-interface/components/Pools/PoolActionOptions.tsx (1)

123-123: ⚠️ Potential issue

Avoid using as any type assertion.

The type assertion Fragment as any bypasses TypeScript's type checking system, which could hide potential runtime errors and make the code harder to maintain. This appears to be a workaround for type compatibility issues with the @headlessui/react Transition component.

Consider these alternatives:

  1. Investigate if there's a version mismatch between TypeScript and @headlessui/react
  2. Use a more specific type assertion that maintains type safety:
-as={Fragment as any}
+as={Fragment as React.ComponentType}
  1. If the issue persists, consider opening an issue in the @headlessui/react repository

Let's check if this is a common pattern in the codebase:

packages/synapse-interface/components/Activity/Transaction/TransactionOptions.tsx (1)

148-148: ⚠️ Potential issue

Consider avoiding any type assertion.

The type assertion Fragment as any bypasses TypeScript's type checking. This could mask potential type incompatibilities with the Transition component from @headlessui/react.

Consider one of these alternatives:

- as={Fragment as any}
+ as={Fragment}

Or explicitly type the component:

import { Fragment, ComponentProps } from 'react'
import { Transition } from '@headlessui/react'

type TransitionRootProps = ComponentProps<typeof Transition.Root>
packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx (2)

164-164: ⚠️ Potential issue

Remove duplicate type assertion pattern

Same issue as above - using Fragment as any bypasses TypeScript's type checking unnecessarily.

Apply the same solution as suggested for line 121.


121-121: Verify @headlessui/react compatibility

The need for type assertions might indicate compatibility issues with the current version of @headlessui/react.

Let's check the package version and available updates:

Also applies to: 164-164

@@ -41,72 +41,75 @@ jobs:
- name: Run tests # Run tests of all packages
run: npx lerna exec npm run ci:lint --parallel

- name: Run spellcheck # Run spellcheck of all packages
run: npx lerna exec npm run ci:spellcheck --parallel
Copy link
Collaborator

Choose a reason for hiding this comment

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

How do we feel about using --if-present flag for the ci:spellcheck script? This should allow to define the script only in the packages we care about the spellcheck and simply ignore the rest of the packages.

Suggested change
run: npx lerna exec npm run ci:spellcheck --parallel
run: npx lerna exec npm run ci:spellcheck --parallel -- --if-present

@github-actions github-actions bot removed M-contracts Module: Contracts 2-reviewers labels Oct 29, 2024
@github-actions github-actions bot added size/m and removed size/l labels Oct 29, 2024
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)
packages/synapse-constants/package.json (1)

Line range hint 43-47: Move build-related packages to devDependencies

The following packages are build tools and should be in devDependencies:

  • @rollup/plugin-commonjs
  • @rollup/plugin-json
  • @rollup/plugin-node-resolve
  • @rollup/plugin-terser
  • @rollup/plugin-url
  • rollup
  • rollup-plugin-typescript2
  • typescript

Apply this diff:

  "dependencies": {
    "@ethersproject/address": "5.7.0",
    "@ethersproject/bignumber": "5.7.0",
-   "@rollup/plugin-commonjs": "^28.0.0",
-   "@rollup/plugin-json": "^6.1.0",
-   "@rollup/plugin-node-resolve": "^15.3.0",
-   "@rollup/plugin-terser": "^0.4.4",
-   "@rollup/plugin-url": "^8.0.2",
    "babel-plugin-transform-export-extensions": "^6.22.0",
    "ethers": "5.7.2",
    "lodash": "^4.17.21",
-   "rollup": "^4.22.4",
-   "rollup-plugin-typescript2": "^0.36.0",
-   "typescript": "^5.3.3"
  },
  "devDependencies": {
+   "@rollup/plugin-commonjs": "^28.0.0",
+   "@rollup/plugin-json": "^6.1.0",
+   "@rollup/plugin-node-resolve": "^15.3.0",
+   "@rollup/plugin-terser": "^0.4.4",
+   "@rollup/plugin-url": "^8.0.2",
+   "rollup": "^4.22.4",
+   "rollup-plugin-typescript2": "^0.36.0",
+   "typescript": "^5.3.3"
  }
.github/workflows/packages.yml (1)

115-115: Add newline at end of file.

Add a newline character at the end of the file to follow POSIX standards and prevent potential issues with certain tools.

     GH_COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
+
🧰 Tools
🪛 yamllint

[error] 115-115: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 55bedbb and c7292bd.

📒 Files selected for processing (6)
  • .github/workflows/packages.yml (2 hunks)
  • docs/bridge/package.json (2 hunks)
  • packages/explorer-ui/package.json (2 hunks)
  • packages/sdk-router/package.json (2 hunks)
  • packages/synapse-constants/package.json (1 hunks)
  • packages/synapse-interface/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • docs/bridge/package.json
  • packages/explorer-ui/package.json
  • packages/sdk-router/package.json
  • packages/synapse-interface/package.json
🧰 Additional context used
🪛 yamllint
.github/workflows/packages.yml

[error] 115-115: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (4)
packages/synapse-constants/package.json (2)

48-50: Skipping comment about moving viem and @codecov/rollup-plugin

A previous review already covers moving these packages to devDependencies.


51-51: Skipping comment about missing cspell command

A previous review already covers adding the cspell command to the ci:spellcheck script.

.github/workflows/packages.yml (2)

44-46: LGTM! Well-implemented spellcheck step.

The implementation correctly uses the --if-present flag, which ensures the spellcheck only runs on packages that have defined the ci:spellcheck script. This is an elegant solution that allows for gradual adoption of spellchecking across packages.


Line range hint 88-89: Reference to past review comment about incorrect step ID.

The issue with incorrect step ID reference was previously identified and the comment is still valid.

🧰 Tools
🪛 yamllint

[warning] 48-48: wrong indentation: expected 4 but found 6

(indentation)

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/packages.yml (2)

Line range hint 88-89: Fix incorrect step ID in file change verification.

The step references a non-existent ID verify-changed-files. This should be updated to match the correct step ID verify-yarn-lock defined earlier in the workflow.

Apply this diff to fix the step ID reference:

- if: steps.verify-changed-files.outputs.files_changed == 'true'
- echo "Changed files: ${{ steps.verify-changed-files.outputs.changed_files }}"
+ if: steps.verify-yarn-lock.outputs.files_changed == 'true'
+ echo "Changed files: ${{ steps.verify-yarn-lock.outputs.changed_files }}"
🧰 Tools
🪛 yamllint

[warning] 48-48: wrong indentation: expected 4 but found 6

(indentation)


115-115: Add missing newline at end of file.

Add a newline character at the end of the file to follow YAML best practices.

🧰 Tools
🪛 yamllint

[error] 115-115: no new line character at the end of file

(new-line-at-end-of-file)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 55bedbb and c7292bd.

📒 Files selected for processing (6)
  • .github/workflows/packages.yml (2 hunks)
  • docs/bridge/package.json (2 hunks)
  • packages/explorer-ui/package.json (2 hunks)
  • packages/sdk-router/package.json (2 hunks)
  • packages/synapse-constants/package.json (1 hunks)
  • packages/synapse-interface/package.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • docs/bridge/package.json
  • packages/explorer-ui/package.json
  • packages/sdk-router/package.json
  • packages/synapse-constants/package.json
  • packages/synapse-interface/package.json
🧰 Additional context used
🪛 yamllint
.github/workflows/packages.yml

[error] 115-115: no new line character at the end of file

(new-line-at-end-of-file)

🔇 Additional comments (2)
.github/workflows/packages.yml (2)

44-46: LGTM! Well-implemented spellcheck step.

The implementation follows best practices with parallel execution and optional package inclusion using --if-present.


Line range hint 110-115: Previous review comments are still applicable.

The issues regarding build failure suppression and environment variable placement have been previously identified and need to be addressed.

🧰 Tools
🪛 yamllint

[error] 115-115: no new line character at the end of file

(new-line-at-end-of-file)

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

🧹 Outside diff range and nitpick comments (2)
docs/bridge/docs/04-Routers/RFQ/API/index.md (2)

16-16: Add a comma for better readability

Add a comma before "and" to separate the independent clauses:

-Solvers are responsible for keeping quotes fresh and implementations of the RFQ relayer should update quotes as often as possible.
+Solvers are responsible for keeping quotes fresh, and implementations of the RFQ relayer should update quotes as often as possible.
🧰 Tools
🪛 LanguageTool

[uncategorized] ~16-~16: Possible missing comma found.
Context: ...vers are responsible for keeping quotes fresh and implementations of the RFQ relayer ...

(AI_HYDRA_LEO_MISSING_COMMA)


74-74: Add comma for better readability

Add a comma before "please" in the MySQL DSN explanation:

-  - `dsn` - the dsn of your database. If using sqlite, this can be a path, if using mysql please see [here](https://dev.mysql.com/doc/connector-odbc/en/connector-odbc-configuration.html) for more information.
+  - `dsn` - the dsn of your database. If using sqlite, this can be a path, if using mysql, please see [here](https://dev.mysql.com/doc/connector-odbc/en/connector-odbc-configuration.html) for more information.
🧰 Tools
🪛 LanguageTool

[typographical] ~74-~74: It seems that a comma is missing.
Context: ...lite, this can be a path, if using mysql please see [here](https://dev.mysql.com/doc/co...

(IF_PLEASE_COMMA)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between c7292bd and 421a343.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (1)
  • docs/bridge/docs/04-Routers/RFQ/API/index.md (4 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/bridge/docs/04-Routers/RFQ/API/index.md

[uncategorized] ~16-~16: Possible missing comma found.
Context: ...vers are responsible for keeping quotes fresh and implementations of the RFQ relayer ...

(AI_HYDRA_LEO_MISSING_COMMA)


[typographical] ~74-~74: It seems that a comma is missing.
Context: ...lite, this can be a path, if using mysql please see [here](https://dev.mysql.com/doc/co...

(IF_PLEASE_COMMA)

🪛 Markdownlint
docs/bridge/docs/04-Routers/RFQ/API/index.md

53-53: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)


55-55: Punctuation: ':'
Trailing punctuation in heading

(MD026, no-trailing-punctuation)

🔇 Additional comments (1)
docs/bridge/docs/04-Routers/RFQ/API/index.md (1)

92-92: LGTM! Good security practice recommendation

Excellent security recommendation about pinning Docker versions in production environments, with a helpful link to available tags.

### API Urls

- Mainnet: `api.synapseprotocol.com/quotes`
- Testnet: `rfq-api-testnet.omnirpc.io`

-
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove empty bullet point

The empty bullet point should be removed as it serves no purpose and violates markdown formatting guidelines.

 - Mainnet: `api.synapseprotocol.com/quotes`
 - Testnet: `rfq-api-testnet.omnirpc.io`
- - 
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
-
- Mainnet: `api.synapseprotocol.com/quotes`
- Testnet: `rfq-api-testnet.omnirpc.io`
🧰 Tools
🪛 Markdownlint

53-53: Expected: 0; Actual: 1
Unordered list indentation

(MD007, ul-indent)

@abtestingalpha abtestingalpha merged commit 65bf288 into master Oct 29, 2024
41 checks passed
@abtestingalpha abtestingalpha deleted the docs/spellcheck branch October 29, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants