diskonaut
diskonaut copied to clipboard
Add GitHub Actions (GHA) CICD workflow
The PR adds a complete GHA CICD workflow:
- includes multiple linux, macos, and windows platform builds and testing
- includes automated packaging and publishing/deployment to GitHub Releases when commit is version tagged
- fully no-touch/automatically generated (when published version tag of the regex form
[Vv]?\d.*) - includes debian packaging
- see example at https://github.com/rivy/rs.diskonaut/releases
- fully no-touch/automatically generated (when published version tag of the regex form
- includes Style testing (both
cargo fmtandcargo clippywarnings); note: warnings add notations but don't break the workflow build- there are about 9 distinct
cargo clippywarnings generated for the current project code
- there are about 9 distinct
- includes test for minimum rust supported version (aka, MinSRV, MSRV, MinRustV); easily removed if not desired
- includes automatic CodeCov coverage reporting (if/when testing is included); also, easily removed if not desired
- NOTE: several of the usual code coverage compiler flags cause test errors for this project so coverage here is using only
-Zprofile, and coverage looks to be skewed (see notes below); I don't have the project knowledge to fix the test failures.
- NOTE: several of the usual code coverage compiler flags cause test errors for this project so coverage here is using only
I've been doing some initial experimentation with your project, and I wanted to contribute some automation that might be helpful to you going forward. This contributed workflow is a slightly modified form based on other GHA CICD workflows that I've designed (and are in current use) for uutils/coreutils, bootandy/dust, Peltoche/lsd, sharkdp/bat, sharkdp/pastel.
I'm happy to make any changes that you would find useful as improvements.
Code coverage notes
For code coverage, the usual flag setting is RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Zpanic_abort_tests -Cpanic=abort', but that combination causes compilation failure with:
error[E0463]: can't find crate for `proc_macro_error_attr`
--> C:\Users\Roy\.cargo\registry\src\github.com-1285ae84e5963aae\proc-macro-error-1.0.2\src\lib.rs:266:9
|
266 | pub use proc_macro_error_attr::proc_macro_error;
| ^^^^^^^^^^^^^^^^^^^^^ can't find crate
error: aborting due to previous error
For more information about this error, try `rustc --explain E0463`.
error: could not compile `proc-macro-error`
To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error[E0463]: can't find crate for `failure_derive`
--> C:\Users\Roy\.cargo\registry\src\github.com-1285ae84e5963aae\failure-0.1.8\src\lib.rs:56:1
|
56 | extern crate failure_derive;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate
error: aborting due to previous error
For more information about this error, try `rustc --explain E0463`.
error: build failed
Using RUSTFLAGS: '-Zprofile -Ccodegen-units=1 -Clink-dead-code -Coverflow-checks=off', tests compile but only one test passes:
...
---- tests::cases::ui::zoom_into_small_files stdout ----
---- tests::cases::ui::zoom_into_small_files stderr ----
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "tests run with disabled concurrency, automatic snapshot name generation is not supported. Consider using the \"backtrace\" feature of insta which tries to recover test names from the call stack."', C:\Users\Roy\.cargo\registry\src\github.com-1285ae84e5963aae\insta-0.16.0\src\runtime.rs:863:22
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
tests::cases::ui::cannot_move_into_small_files
tests::cases::ui::cant_delete_file_with_term_too_small
tests::cases::ui::clear_selection_when_moving_off_screen_edges
tests::cases::ui::delete_file
tests::cases::ui::delete_file_no_confirmation
tests::cases::ui::delete_file_press_n
tests::cases::ui::delete_folder
tests::cases::ui::delete_folder_no_confirmation
tests::cases::ui::delete_folder_small_window
tests::cases::ui::delete_folder_small_window_no_confirmation
tests::cases::ui::delete_folder_with_multiple_children
tests::cases::ui::delete_folder_with_multiple_children_no_confirmation
tests::cases::ui::eleven_files
tests::cases::ui::empty_folder
tests::cases::ui::enter_folder
tests::cases::ui::enter_folder_medium_width
tests::cases::ui::enter_folder_small_width
tests::cases::ui::enter_largest_folder_with_no_selected_tile
tests::cases::ui::esc_to_go_up
tests::cases::ui::files_with_size_zero
tests::cases::ui::medium_width
tests::cases::ui::minimum_tile_sides
tests::cases::ui::move_down_and_enter_folder
tests::cases::ui::move_left_and_enter_folder
tests::cases::ui::move_right_and_enter_folder
tests::cases::ui::move_up_and_enter_folder
tests::cases::ui::noop_when_entering_file
tests::cases::ui::noop_when_pressing_esc_at_base_folder
tests::cases::ui::pressing_delete_with_no_selected_tile
tests::cases::ui::small_files
tests::cases::ui::small_files_with_x_as_zero
tests::cases::ui::small_files_with_y_as_zero
tests::cases::ui::small_width
tests::cases::ui::small_width_long_folder_name
tests::cases::ui::too_small_height
tests::cases::ui::too_small_width_five
tests::cases::ui::too_small_width_four
tests::cases::ui::too_small_width_one
tests::cases::ui::too_small_width_three
tests::cases::ui::too_small_width_two
tests::cases::ui::two_large_files_one_small_file
tests::cases::ui::zoom_into_small_files
test result: FAILED. 1 passed; 42 failed; 0 ignored; 0 measured; 0 filtered out
Using RUSTFLAGS: '-Zprofile -Clink-dead-code -Coverflow-checks=off', tests compile but two test failures occur:
failures:
tests::cases::ui::delete_file_no_confirmation
tests::cases::ui::delete_folder_no_confirmation
test result: FAILED. 41 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
Hey @rivy - I'm sorry it's taking me some time to get to this. Going through some busy days. I promise to review this soon.
No worries. I appreciate the response, but take your time. I'm in no rush. Let me know if you need changes.
Hey @rivy - thanks for your patience! I'm finally getting to this. :)
Because of the nature of this change, I think it's going to require some back-and-forth between us. I hope you're up to it, and I promise not to take so long to reply from now on.
So a few things:
- I'm not a big fan of test coverage reporting, in all honesty. While it's a good tool for manually checking what one might be missing in their tests, I feel that publishing it can create blind-spots and overconfidence. So I think I'd rather not include it.
- Elsewhere, I've seen that it's possible to trigger releases manually rather than with a commit regex. I greatly prefer that approach because it gives me finer control over the release process. Would changing it be a great deal of work?
- I really like that the releases would include prebuilt binaries for windows and mac. I feel that would be very meaningful to new users wanting to try out diskonaut. I think I would prefer to have just 1 file per OS in order to make things simpler for those looking to download the right version. What do you think?
- I'm aware of the clippy warnings unfortunately. :/ That's why I didn't include the check originally when releasing diskonaut. Some of them aren't very trivial to fix iirc. Could we include this check in a disabled mode somehow and when I or someone else gets to fixing them we can enable it? (If you're feeling adventurous and want to fix them, I'd be happy to guide you through what I think needs to be done - but no stress of course).
Otherwise, this looks really cool and I'm looking forward to using it when releasing. Thanks for doing this!
Sorry for the year-long delay! Your reply got lost in the stack and I just saw it recently when I was reviewing old PRs.
I've gone ahead and made changes following your requests.
- I'm not a big fan of test coverage reporting, in all honesty. While it's a good tool for manually checking what one might be missing in their tests, I feel that publishing it can create blind-spots and overconfidence. So I think I'd rather not include it.
Disabled using comments in a specific and described commit so that it's there if you want to re-enable it in the future.
As an aside, there is also a style spell-check, using cspell, that I've left in (again, disabled by comments) for the possibility that you might want to use it in the future. It would require some configuration work on your part.
- Elsewhere, I've seen that it's possible to trigger releases manually rather than with a commit regex. I greatly prefer that approach because it gives me finer control over the release process. Would changing it be a great deal of work?
This workflow already supports controlled (ie, manual) releases via tags. It's not gated on commit matches but upon tag matches. If you don't push a release tag (ie, matches /[vV]?[0-9].*/), no release will be deployed. So, you will always have full control of the releases.
Using GHA workflow dispatch to enable manually triggering the CICD workflow via UI would require an involved design refactor and, likely, a lot of redundant code without adding any capabilities.
- ref: https://docs.github.com/en/actions/learn-github-actions/workflow-syntax-for-github-actions
- ref: https://blog.knoldus.com/manual-trigger-in-github-actions @@ https://archive.is/wVfQ8
I do think you'll find the current implementation will meet your request.
- I really like that the releases would include prebuilt binaries for windows and mac. I feel that would be very meaningful to new users wanting to try out diskonaut. I think I would prefer to have just 1 file per OS in order to make things simpler for those looking to download the right version. What do you think?
I added a "withhold" parameter to the build matrix which suppresses publishing those marked packages for release. The configuration offered here publishes the i686 (x32) packages (as more widely usable) for Windows and Linux while suppressing the respective x86_64 (x64) packages; see https://github.com/rivy/rs.diskonaut/releases. Alternatively, you could just comment out the configurations in the job matrix to avoid building those packages altogether. But, IMO, building and testing those configurations makes your project much more robust, so I'd leave them to build/test.
- I'm aware of the clippy warnings unfortunately. :/ That's why I didn't include the check originally when releasing diskonaut. Some of them aren't very trivial to fix iirc. Could we include this check in a disabled mode somehow and when I or someone else gets to fixing them we can enable it? (If you're feeling adventurous and want to fix them, I'd be happy to guide you through what I think needs to be done - but no stress of course).
My OCD was tingling and wouldn't let me leave the warnings in, so I fixed them all. The code now compiles without warnings and cargo clippy --all-targets has no complaints. The fixes are in separate commits so that you can review them easily.
I also rebased the changes on top of the current main branch.
Hope you like it! Let me know if you need help with any changes.
I'll keep a better eye out this time.
I guess we missed each other... closing.