feat(synapse-constants): adds preinstall step #3269
Conversation
abtestingalpha
requested review from
trajan0x and
bigboydiamonds
as code owners
WalkthroughThe pull request includes updates to documentation files and a package configuration file. The Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
|
Deploying sanguine-fe with
|
| Latest commit: |
77c2848
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8833cc13.sanguine-fe.pages.dev |
| Branch Preview URL: | https://synapse-constants-preinstall.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
packages/explorer-ui/README.md (2)
5-6: Approve formatting change and suggest TODO item revision.The formatting change for the TODO items improves readability. However, the first TODO item "add readme describing explorer ui" seems redundant as this file is already a README for the Explorer UI.
Consider removing or updating the first TODO item to be more specific about what additional information is needed in the README.
8-18: Suggest adding information about the preinstall step.The PR objectives mention adding a preinstall step for @synapsecns/synapse-constants, but this isn't reflected in the README. Consider adding a note about this preinstall step if it's relevant to developers working on the Explorer UI.
docs/bridge/README.md (2)
13-13: Great addition of syntax highlightingThe addition of
bashsyntax highlighting to the code blocks is an excellent improvement for readability.Consider removing the dollar signs ($) before the commands as suggested by Markdownlint. This can improve copy-paste functionality for users. For example:
yarninstead of:
$ yarnAlso applies to: 19-19, 27-27
32-39: Excellent addition of the "Serve" sectionThe new "Serve" section is a valuable addition to the documentation. It clearly explains the purpose of this step and provides the necessary command.
Consider the following minor improvements:
- Remove the dollar sign ($) before the command for consistency with the previous suggestion.
- Add a brief explanation of what the searchable index is used for, to provide more context to users.
Example:
### Serve This step is needed to create a searchable index, which enables users to search through the documentation. ```bash yarn serve<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint</summary><blockquote> 38-38: null Dollar signs used before commands without showing output (MD014, commands-show-output) </blockquote></details> </details> </blockquote></details> <details> <summary>packages/synapse-constants/package.json (1)</summary><blockquote> `30-30`: **Approve: Preinstall script addition improves package consistency** The addition of the `"preinstall": "yarn build"` script is a good improvement. It ensures that the package is always built before installation, which aligns with the PR objective of eliminating the need for local installation of `@synapsecns/synapse-constants`. This change will streamline the installation process for users and ensure consistency across different environments. Consider adding a comment in the package.json to explain the purpose of the preinstall script, for example: ```diff "scripts": { ... + // Ensures the package is built before installation "preinstall": "yarn build", "prepublish": "yarn build", ... },This will help other developers understand the reasoning behind this script in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- docs/bridge/README.md (1 hunks)
- packages/explorer-ui/README.md (1 hunks)
- packages/synapse-constants/package.json (1 hunks)
🧰 Additional context used
🪛 Markdownlint
docs/bridge/README.md
14-14: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
20-20: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
28-28: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
38-38: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
🔇 Additional comments (4)
packages/explorer-ui/README.md (2)
8-18: Approve new "To get started" instructions.The new instructions are clear, concise, and align with the project's use of yarn as the package manager. This simplification will help new contributors get started more easily.
8-18: Request clarification on project setup change.The removal of Create React App instructions suggests a possible change in the project setup. Could you please clarify if there have been any significant changes to the project structure or build process that should be documented here?
docs/bridge/README.md (2)
8-8: LGTM: Improved readabilityThe addition of a blank line after the TODO comment enhances the document's readability by providing better visual separation.
Line range hint
1-39: Overall excellent improvements to the documentationThe changes made to this README file significantly enhance its clarity, structure, and completeness. The improved formatting, addition of syntax highlighting, and the new "Serve" section all contribute to making the documentation more user-friendly and informative.
These improvements align well with the PR objectives of enhancing the documentation. Great job on making the installation and usage process clearer for users of the Docusaurus-based website.
🧰 Tools
🪛 Markdownlint
14-14: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
20-20: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
28-28: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
38-38: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3269 +/- ##
=============================================
Coverage 31.76937% 31.76937%
=============================================
Files 427 427
Lines 28496 28496
Branches 82 82
=============================================
Hits 9053 9053
Misses 18597 18597
Partials 846 846
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Bundle ReportChanges will decrease total bundle size by 3.91MB (-10.97%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Description
Adds preinstall step to avoid requiring local installation of
@synapsecns/synapse-constantsSummary by CodeRabbit
Documentation
README.mdfor the Bridge project to enhance clarity on using Docusaurus for API documentation, including a new "Serve" section.README.mdto replace the Create React App section with a new "To get started" section.Chores
6cd79f2: docs preview link
6cd79f2: explorer-ui preview link