fixing restapi gh actions flake #3139
Conversation
Defi-Moses
requested review from
bigboydiamonds,
abtestingalpha and
trajan0x
as code owners
WalkthroughThe changes made to the Changes
Sequence Diagram(s)(No sequence diagrams generated as the changes do not warrant a control flow visualization.) Poem
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
Documentation and Community
Early access features: enabledWe are currently testing the following features in early access:
Note:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (1)
- packages/rest-api/package.json (3 hunks)
Additional comments not posted (1)
packages/rest-api/package.json (1)
11-11: Verify the necessity of the 'build:slither' scriptTo ensure that the
build:slitherscript is intentional and correctly implemented, please verify its usage across the codebase.Run the following script to search for occurrences of
build:slither:Verification successful
Empty 'build:slither' script is likely a placeholder
The 'build:slither' script with an empty command (" ") appears to be a standardized entry across multiple package.json files in the project. For non-contract packages like rest-api, it's likely a placeholder or part of a consistent package structure. Slither analysis seems to be primarily used for smart contracts and is possibly integrated into the CI/CD process rather than local builds for most packages.
While the script doesn't have any negative impact, it's not actively used in the rest-api package. Consider either:
- Removing it if it's unnecessary for this package.
- Adding a comment explaining its purpose as a placeholder.
- Implementing Slither analysis if it becomes relevant for this package in the future.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to 'build:slither' in the codebase. # Test: Find all instances where 'build:slither' is used. rg --type json '"build:slither"'Length of output: 786
Script:
#!/bin/bash # Search for references to 'build:slither' in non-JSON files rg --type-not json 'build:slither' # Search for README files that might explain the build process fd -i readme.md # Search for Slither configuration files or mentions rg -i 'slither' # Check for CI/CD configuration files fd -e yml -e yamlLength of output: 6970
| "dev": "nodemon --watch src --exec ts-node src/app.ts", | ||
| "start": "node dist/app.js", | ||
| "lint:fix": "eslint src/**/*.ts --fix", | ||
| "lint:check": "eslint . --max-warnings=0", | ||
| "ci:lint": "npm run lint:check", | ||
| "test": "jest", | ||
| "test:coverage": "echo 'No tests defined.'", | ||
| "postinstall": "npm run build" | ||
| "test:coverage": "echo 'No tests defined.'" |
There was a problem hiding this comment.
'test:coverage' script provides no functional value
The test:coverage script currently echoes 'No tests defined.', which may be misleading. Consider implementing actual test coverage reporting or removing this script to prevent confusion.
Apply this diff to remove the non-functional script:
- "test:coverage": "echo 'No tests defined.'"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.
| "test:coverage": "echo 'No tests defined.'" |
| @@ -8,14 +8,14 @@ | |||
| "scripts": { | |||
| "build": "tsc", | |||
| "build:go": " ", | |||
| "build:slither": " ", | |||
There was a problem hiding this comment.
Empty 'build:slither' script may cause confusion
The build:slither script has been added with an empty command (" "). This could lead to confusion or unintended behavior if someone tries to execute it. If the script is not yet implemented, consider removing it until it's ready or providing a meaningful command.
Apply this diff to remove the empty script:
- "build:slither": " ",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.
| "build:slither": " ", |
| @@ -26,7 +26,9 @@ | |||
| "ethers": "5.7.2", | |||
| "express": "^4.18.2", | |||
| "express-validator": "^7.2.0", | |||
| "lodash": "^4.17.21" | |||
| "lodash": "^4.17.21", | |||
There was a problem hiding this comment.
Duplicate 'lodash' dependency
The lodash package is listed in both dependencies and devDependencies. This duplication can cause unnecessary bloat and potential version conflicts. Determine if lodash is required at runtime or only during development, and include it in the appropriate section.
If lodash is only needed during development, remove it from dependencies:
- "lodash": "^4.17.21",Committable suggestion was skipped due to low confidence.
| "supertest": "^6.3.3", | ||
| "typescript": "^4.8.3" |
There was a problem hiding this comment.
Move 'supertest' and 'typescript' back to 'devDependencies'
supertest and typescript are typically used for development and testing purposes. Including them in dependencies will install them in production environments unnecessarily, increasing the application's footprint.
Apply this diff to move them back to devDependencies:
- "supertest": "^6.3.3",
- "typescript": "^4.8.3"Add them to devDependencies if they're not already present:
+ "supertest": "^6.3.3",
+ "typescript": "^4.8.3"Committable suggestion was skipped due to low confidence.
Deploying sanguine-fe with
|
| Latest commit: |
8e974b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83e0b461.sanguine-fe.pages.dev |
| Branch Preview URL: | https://bug-api-gh-actions.sanguine-fe.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3139 +/- ##
===================================================
- Coverage 38.22166% 37.94554% -0.27612%
===================================================
Files 423 418 -5
Lines 24371 24240 -131
Branches 119 82 -37
===================================================
- Hits 9315 9198 -117
+ Misses 14317 14303 -14
Partials 739 739
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Edits the rest api to fix a gh actions flake
Summary by CodeRabbit
New Features
supertestfor improved API endpoint testing.typescriptpackage for better type safety.Chores
package.jsonfor better project structure.