Skip to content
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

extend ci #10

Merged
merged 11 commits into from
Mar 21, 2024
Merged

extend ci #10

merged 11 commits into from
Mar 21, 2024

Conversation

kod-kristoff
Copy link
Owner

  • ci: copy from jonhoo/rust-ci-conf
  • ci: remove nostd safety

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @kod-kristoff - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Hard-coded secret detected. (link)

General suggestions:

  • Review the pinning of GitHub Actions to specific commits or tags to ensure stability and reproducibility in CI workflows.
  • Evaluate the security implications of introducing third-party GitHub Actions and ensure they comply with the project's security policies.
  • Consider the impact of treating deprecation warnings as errors in CI on the development workflow and dependency update strategy.
  • Ensure sensitive information, such as tokens, is securely managed using GitHub Secrets rather than being hard-coded in workflow files.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🔴 Security: 1 blocking issue, 2 other issues
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

Comment on lines +32 to +33
- name: Install ${{ matrix.toolchain }}
uses: dtolnay/rust-toolchain@master
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code_refinement): Consider pinning the rust-toolchain action to a specific commit or tag.

Using a branch reference like 'master' can lead to unexpected changes in your CI pipeline when the action is updated. Pinning to a specific commit or tag ensures reproducibility and stability.

toolchain: [stable, beta]
steps:
- uses: actions/checkout@v4
with:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 suggestion (security): Ensure that the usage of third-party actions aligns with security policies.

Using third-party GitHub Actions like 'codecov/codecov-action@v4' introduces external dependencies into your CI/CD pipeline. It's important to review and ensure these actions comply with your project's security policies and practices.

Comment on lines +57 to +58
env:
RUSTFLAGS: -D deprecated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (code_refinement): Consider the implications of treating deprecation warnings as errors in CI.

Using -D deprecated treats all deprecation warnings as errors, which can be beneficial for keeping the codebase modern and clean. However, it might also introduce friction when dependencies deprecate features. Ensure this aligns with your development workflow and dependency update strategy.

Comment on lines 30 to 155
# direct dependencies of this crate, while selecting the maximal versions for the transitive
# dependencies. Alternatively, you can add a line in your Cargo.toml to artificially increase
# the minimal dependency, which you do with e.g.:
# ```toml
# # for minimal-versions
# [target.'cfg(any())'.dependencies]
# openssl = { version = "0.10.55", optional = true } # needed to allow foo to build with -Zminimal-versions
# ```
# The optional = true is necessary in case that dependency isn't otherwise transitively required
# by your library, and the target bit is so that this dependency edge never actually affects
# Cargo build order. See also
# https://github.com/jonhoo/fantoccini/blob/fde336472b712bc7ebf5b4e772023a7ba71b2262/Cargo.toml#L47-L49.
# This action is run on ubuntu with the stable toolchain, as it is not expected to fail
runs-on: ubuntu-latest
name: ubuntu / stable / minimal-versions
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install stable
uses: dtolnay/rust-toolchain@stable
- name: Install nightly for -Zminimal-versions
uses: dtolnay/rust-toolchain@nightly
- name: rustup default stable
run: rustup default stable
- name: cargo update -Zminimal-versions
run: cargo +nightly update -Zdirect-minimal-versions
- name: cargo test
run: cargo test --locked --all-features --all-targets
os-check:
# run cargo test on mac and windows
runs-on: ${{ matrix.os }}
name: ${{ matrix.os }} / stable
strategy:
fail-fast: false
matrix:
os: [macos-latest, windows-latest]
steps:
# if your project needs OpenSSL, uncomment this to fix Windows builds.
# it's commented out by default as the install command takes 5-10m.
# - run: echo "VCPKG_ROOT=$env:VCPKG_INSTALLATION_ROOT" | Out-File -FilePath $env:GITHUB_ENV -Append
# if: runner.os == 'Windows'
# - run: vcpkg install openssl:x64-windows-static-md
# if: runner.os == 'Windows'
- uses: actions/checkout@v4
with:
submodules: true
- name: Install stable
uses: dtolnay/rust-toolchain@stable
- name: cargo generate-lockfile
if: hashFiles('Cargo.lock') == ''
run: cargo generate-lockfile
- name: cargo test
run: cargo test --locked --all-features --all-targets
coverage:
# use llvm-cov to build and collect coverage and outputs in a format that
# is compatible with codecov.io
#
# note that codecov as of v4 requires that CODECOV_TOKEN from
#
# https://app.codecov.io/gh/<user or org>/<project>/settings
#
# is set in two places on your repo:
#
# - https://github.com/jonhoo/guardian/settings/secrets/actions
# - https://github.com/jonhoo/guardian/settings/secrets/dependabot
#
# (the former is needed for codecov uploads to work with Dependabot PRs)
#
# PRs coming from forks of your repo will not have access to the token, but
# for those, codecov allows uploading coverage reports without a token.
# it's all a little weird and inconvenient. see
#
# https://github.com/codecov/feedback/issues/112
#
# for lots of more discussion
runs-on: ubuntu-latest
name: ubuntu / stable / coverage
steps:
- uses: actions/checkout@v4
with:
submodules: true
- name: Install stable
uses: dtolnay/rust-toolchain@stable
with:
components: llvm-tools-preview
- name: cargo install cargo-llvm-cov
uses: taiki-e/install-action@cargo-llvm-cov
- name: cargo generate-lockfile
if: hashFiles('Cargo.lock') == ''
run: cargo generate-lockfile
- name: cargo llvm-cov
run: cargo llvm-cov --locked --all-features --lcov --output-path lcov.info
- name: Record Rust version
run: echo "RUST=$(rustc --version)" >> "$GITHUB_ENV"
- name: Upload to codecov.io
uses: codecov/codecov-action@v4
with:
fail_ci_if_error: true
token: ${{ secrets.CODECOV_TOKEN }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Hard-coded secret detected.

It's recommended to avoid hard-coding secrets and tokens in workflow files. Consider using GitHub Secrets to store sensitive information securely.

Copy link

codecov bot commented Mar 20, 2024

Codecov Report

Attention: Patch coverage is 98.92473% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 91.3%. Comparing base (e82e806) to head (bdb44b9).
Report is 14 commits behind head on main.

Additional details and impacted files
Files Coverage Δ
...g_platform/auctions/src/domain/entities/auction.rs 100.0% <ø> (ø)
...form/auctions/src/domain/entities/auction/tests.rs 100.0% <100.0%> (ø)
auctioning_platform/webapp/src/main.rs 0.0% <0.0%> (ø)

... and 9 files with indirect coverage changes

@kod-kristoff kod-kristoff merged commit e0f172e into main Mar 21, 2024
18 checks passed
@kod-kristoff kod-kristoff deleted the extend-ci branch March 21, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant