You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I would like to propose some ci/cd enhancements and clarify several points regarding GitHub workflows, dependabot configurations, and general project setup.
Release and Dev Builds
Clarification Needed: Do you want to upload compiled binaries during CI builds to a service that provides a web page where you can download zipped binaries? These dev/release builds can be uploaded for each branch in the repository and stored for a specified period, such as 10 or 15 days, as per your preference.
Docker images: Docker images will be needed to compile binaries. Do you have an official page on Docker Hub for these images?
Secrets: GitHub can store server private keys securely. We need to coordinate on setting up Docker/build services, as workflows will use GitHub secrets to access the server for uploading binary zips, for example.
GitHub Workflows YMLs List
pr-lint.yml:
Could use the semantic pull request action to check prefixes in PR names (e.g., feat(feature name):, fix(some fixed feature name):). It also can check PR labels, ensuring that each PR specifies a status label (e.g., under review or in progress). i suggest to add these lables
fmt-and-lint.yml:
Runs cargo fmt and cargo clippy.
test.yml:
Executes tests for the project.
dep-audit.yml:
Runs commands:
cargo udeps makes sure there are no unused dependencies
cargo deny check bans no new dependencies duplicates appear. Run the following check
cargo deny check advisories dependencies do not have known vulnerabilities. If they do, update them.
dev-build.yml:
Compiles debug mode binaries and uploads them to the build service.
release-build.yml:
Compiles release binaries and uploads them to the build service.
Chaincash-RS Releases
For the Releases section (https://github.com/ChainCashLabs/chaincash-rs/releases) I suggest to upload release zips to VirusTotal for security checks.
Well I suggest to add new releases manually, by upploading zips to virustotal, providing checksum, adding release description etc. We can discuss the first release draft later.
Clippy in CI
Proposal: I suggest using cargo clippy -- -D warnings in the CI fmt-lint pipeline. Enforcing warnings as errors helps maintain clean, readable code and catches potential issues early.
Compilation Targets
Clarification Needed: The targets list provided in deny.toml (https://github.com/ChainCashLabs/chaincash-rs/blob/master/deny.toml) seems to be the actual compilation target list. Could you confirm if this list is comprehensive? Are there any plans to include WASM targets or any other platforms?
Rust Toolchain Version
Proposal: Would you prefer to provide a rust-toolchain.toml file specifying the exact rustc version to use? This ensures consistency across all development and CI environments, reducing the chances of version-related issues.
Dependabot Configuration
Capabilities: Dependabot can open PRs for dependency updates, which can then be reviewed and merged manually. It can also notify you of any security vulnerabilities in dependencies.
Branch to Check: Which branch should Dependabot monitor for dependency updates? Typically, this would be the main or dev branch. Are there plans to add a dev branch?
Merge Restrictions: If Dependabot alerts about dependency issues, do you want to enforce merge restrictions to the master branch until these issues are resolved?
I can provide a Dependabot configuration in PR with workflows, so you can review it. It's important to be careful with allowing Dependabot to open PRs automatically, as it can generate many PRs, potentially overwhelming the team. However, it remains useful for notifying about security vulnerabilities.
Documentation Comments
Policy on Documentation: While it is beneficial to have as many doc comments as possible, enforcing strict deny rules for missing doc comments might be too stringent. I suggest recommending doc comments in contributors.md and review.md guides. Contributors should document public APIs, and reviewers should ensure this documentation is in place. This approach helps future contributors understand and use the codebase more effectively.
The text was updated successfully, but these errors were encountered:
On checksum, is it easy to get reproducible build with Rust? With Scala / Java it seems to be very tricky (as compiler is injecting its version and timestamps into binaries).
enforcing strict deny rules for missing doc comments might be too stringent. I suggest recommending doc comments in contributors.md and review.md guides
I would like to propose some ci/cd enhancements and clarify several points regarding GitHub workflows,
dependabot
configurations, and general project setup.Release and Dev Builds
GitHub Workflows YMLs List
pr-lint.yml:
Could use the semantic pull request action to check prefixes in PR names (e.g.,
feat(feature name):
,fix(some fixed feature name):
). It also can check PR labels, ensuring that each PR specifies a status label (e.g.,under review
orin progress
). i suggest to add these lablesfmt-and-lint.yml:
Runs
cargo fmt
andcargo clippy
.test.yml:
Executes tests for the project.
dep-audit.yml:
Runs commands:
cargo udeps
makes sure there are no unused dependenciescargo deny check bans
no new dependencies duplicates appear. Run the following checkcargo deny check advisories
dependencies do not have known vulnerabilities. If they do, update them.dev-build.yml:
Compiles debug mode binaries and uploads them to the build service.
release-build.yml:
Compiles release binaries and uploads them to the build service.
Chaincash-RS Releases
For the
Releases
section (https://github.com/ChainCashLabs/chaincash-rs/releases) I suggest to upload release zips to VirusTotal for security checks.Well I suggest to add new releases manually, by upploading zips to virustotal, providing checksum, adding release description etc. We can discuss the first release draft later.
Clippy in CI
cargo clippy -- -D warnings
in the CI fmt-lint pipeline. Enforcing warnings as errors helps maintain clean, readable code and catches potential issues early.Compilation Targets
deny.toml
(https://github.com/ChainCashLabs/chaincash-rs/blob/master/deny.toml) seems to be the actual compilation target list. Could you confirm if this list is comprehensive? Are there any plans to include WASM targets or any other platforms?Rust Toolchain Version
rust-toolchain.toml
file specifying the exactrustc
version to use? This ensures consistency across all development and CI environments, reducing the chances of version-related issues.Dependabot Configuration
main
ordev
branch. Are there plans to add adev
branch?master
branch until these issues are resolved?I can provide a Dependabot configuration in PR with workflows, so you can review it. It's important to be careful with allowing Dependabot to open PRs automatically, as it can generate many PRs, potentially overwhelming the team. However, it remains useful for notifying about security vulnerabilities.
Documentation Comments
deny
rules for missing doc comments might be too stringent. I suggest recommending doc comments incontributors.md
andreview.md
guides. Contributors should document public APIs, and reviewers should ensure this documentation is in place. This approach helps future contributors understand and use the codebase more effectively.The text was updated successfully, but these errors were encountered: