Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 & Fix: Bitcoin Cosmos SDK Prototype is now onboarded and able to broadcast with cosmos-sdk-broadcast.sh and build, go, and run upgrades! #23171
Feat & Fix: Bitcoin Cosmos SDK Prototype is now onboarded and able to broadcast with cosmos-sdk-broadcast.sh and build, go, and run upgrades! #23171
Changes from all commits
8452d68
57d9b6e
22d8eab
1df449c
3e82d17
901a5ac
b3c5138
f960bf7
f3b15fe
388668e
b936116
77cf283
bdcdd3d
48f115b
2212217
b57c1f9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
🛠️ Refactor suggestion
Consider multi-platform testing requirements
Running only on Ubuntu might not be sufficient for a Bitcoin integration that needs to work across different platforms.
Consider adding more platforms:
📝 Committable suggestion
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.
💡 Codebase verification
The
configure
script is missing and the workflow needs to be updatedThis is primarily a Go-based Cosmos SDK project. While it contains some C/C++ code (mainly in the secp256k1 library), the
configure
script doesn't exist in the root directory. The C/C++ files are part of a vendored dependency incrypto/keys/secp256k1/
which has its own build system..github/workflows/c-cpp.yml
needs to be updated as it incorrectly assumes the presence of a./configure
script in the root directory🔗 Analysis chain
Verify the existence of configure script
The workflow assumes the presence of a
./configure
script, which is typical for C/C++ projects but may not exist in a Cosmos SDK project.Let's verify the configure script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 97
Script:
Length of output: 5908
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
Sorry, something went wrong.
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.
@bearycool11 I see that the C/C++ workflow file hasn't been updated yet. It still contains the
./configure
andmake
commands that aren't suitable for this project. Since this is primarily a Go-based Cosmos SDK project, would you like me to suggest an updated version of the workflow that's better aligned with the project's structure? The workflow could either:crypto/keys/secp256k1/
directoryWhich approach would you prefer?
🔗 Analysis chain
@bearycool11 Welcome back! 👋 I noticed you asked earlier if new commit files helped. Let me check the current state of the C/C++ workflow file to see if the issues we discussed have been addressed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 432
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.
🛠️ Refactor suggestion
Enhance workflow with proper dependency management and artifacts
The current workflow lacks several critical components needed for a robust CI pipeline:
Apply these enhancements:
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.
💡 Codebase verification
Standard make targets are not present in this repository
The repository contains multiple Makefiles, but none of them implement the standard targets (
make
,make check
,make distcheck
) used in the workflow. Instead, the Makefiles are Go-oriented and contain targets like:test
for running Go testsbuild
for building Go binariescodegen
for generating protobuf filesformat
for code formattingThe workflow needs to be updated to use the appropriate make targets for this Go-based project.
🔗 Analysis chain
Verify make targets and build system
The workflow assumes standard make targets (
make
,make check
,make distcheck
) which may not be appropriate for a Cosmos SDK project.Let's verify the make targets:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 143
Script:
Length of output: 9679