-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
chain-mon
restructuring
#10246
chain-mon
restructuring
#10246
Conversation
WalkthroughWalkthroughThe recent changes aim to enhance the project structure and functionality of blockchain monitoring and messaging systems. These updates involve optimizing import paths for better organization, reorganizing export locations for improved accessibility, and strengthening testing capabilities for cross-domain messaging through the introduction of new functions and enhancements in failure handling mechanisms. Changes
Recent Review DetailsConfiguration used: .coderabbit.yml Files selected for processing (1)
Additional comments not posted (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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
2cb9ea6
to
478cfb6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ops/docker/Dockerfile.packages
would needs to change according to the new structure of the folders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
initialized-upgraded-mon should potentially be an internal service, as it was developed recently and is used internally, cc @zchn
but otherwise lgtm
Sgtm |
Also need to update the code owner path for balance mon and add initialized-upgraded-mon unless it is owned by security : Line 3 in 613e5dc
|
Semgrep found 13
Prefer Semgrep found 5
No |
There was a problem hiding this 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
Out of diff range and nitpick comments (1)
ops/docker/Dockerfile.packages (1)
98-98
: Consider updating the comment about deleting unused images to reflect current usage or plans.This comment suggests that there are still unused images that might be cleaned up. It would be beneficial to either update this comment to reflect the current state of the project or to actually remove any unused Docker images to streamline the Dockerfile.
Description
Restructuring
Chain-mon
with the new architecture and pass the test accordingly.