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

modify ci.yml to see if it will turn off code coverage in the reedline repo #710

Merged
merged 4 commits into from
Jan 21, 2024

Conversation

stormasm
Copy link
Contributor

I find reviewing PRs with the code coverage turned on incredibly annoying and hard to understand
all of the other code that I need to understand what has changed...

Unless someone really feels strongly about having it in there I am going to remove it....

Thanks !

Copy link

codecov bot commented Jan 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9ca229d) 47.59% compared to head (87cbf22) 47.59%.

❗ Current head 87cbf22 differs from pull request most recent head 39c3a68. Consider uploading reports for the commit 39c3a68 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #710   +/-   ##
=======================================
  Coverage   47.59%   47.59%           
=======================================
  Files          47       47           
  Lines        9312     9312           
=======================================
  Hits         4432     4432           
  Misses       4880     4880           

@fdncred
Copy link
Collaborator

fdncred commented Jan 20, 2024

I'm wondering if we can disable it without deleting the file just in case we want to easily enable it again?

There a action to upload the coverage results in the ci.yml file too.

@stormasm
Copy link
Contributor Author

Here is a copy of the codecov.yml file

coverage:
  status:
    project:
      default:
        informational: true
    patch:
      default:
        informational: true

comment:
  layout: reach, diff, files
  behavior: default
  require_base: yes
  require_head: yes
  after_n_builds: 5 # Our test matrix currently has 5 configurations

@stormasm
Copy link
Contributor Author

Here is a copy of the associated ci.yml file

on:
  pull_request:
  push: # Run CI on the main branch after every merge. This is important to fill the GitHub Actions cache in a way that pull requests can see it
    branches:
      - main

name: continuous-integration

jobs:
  build-lint-test:
    strategy:
      fail-fast: false
      matrix:
        platform: [ubuntu-latest]
        rust:
          - stable
        # Define the feature sets that will be built here (for caching you define a separate name)
        style: [bashisms, default, sqlite, basqlite, external_printer]
        include:
          - style: bashisms
            flags: "--features bashisms"
          - style: external_printer
            flags: "--features external_printer"
          - style: default
            flags: ""
          - style: sqlite
            flags: "--features sqlite"
          - style: basqlite
            flags: "--features bashisms,sqlite"

    runs-on: ${{ matrix.platform }}

    steps:
      - uses: actions/checkout@v3

      - name: Setup Rust toolchain
        uses: actions-rust-lang/setup-rust-toolchain@v1.3.4
      - name: Setup nextest
        uses: taiki-e/install-action@nextest
      - name: Setup cargo-llvm-cov
        uses: taiki-e/install-action@cargo-llvm-cov

      - name: Rustfmt
        run: cargo fmt --all -- --check

      - name: Clippy
        run: cargo clippy ${{ matrix.flags }} --all-targets --all -- -D warnings


      - name: Tests
        run: cargo llvm-cov nextest --all ${{ matrix.flags }} --lcov --output-path lcov.info

      - name: Doctests
        run: cargo test --doc ${{ matrix.flags }}

      - name: Upload coverage
        uses: codecov/codecov-action@v3
        with:
          files: lcov.info

@stormasm
Copy link
Contributor Author

Note here is another copy of both files for archival reasons in case we want to change back the settings...

https://github.com/stormasm/nunotes/tree/main/archive

@stormasm
Copy link
Contributor Author

For starters I will just attempt to modify the ci.yml action without actually deleting the codecov.yml file to see if that works and turns off code coverage...

If this doesn't work then we will remove the codecov.yml file...

I wish there was a setting that individuals could set to toggle code coverage on and off when you are looking at PRs.

Then you can review the code with out seeing all of the extraneous code coverage messages which get in the way of trying to understand the code changes...

@stormasm stormasm merged commit 7255741 into nushell:main Jan 21, 2024
6 checks passed
@stormasm stormasm deleted the remove-codecov branch January 21, 2024 03:09
@stormasm stormasm changed the title remove codecov.yml in the reedline repo modify ci.yml to see if it will turn off code coverage in the reedline repo Jan 21, 2024
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.

2 participants