emmercm / igir

🕹 A video game ROM collection manager to help filter, sort, patch, archive, and report on collections on any OS.
https://igir.io/
GNU General Public License v3.0
328 stars 16 forks source link

Fix: Update clean dry run to use info logging instead of debug #978

Closed PhasecoreX closed 4 months ago

PhasecoreX commented 5 months ago

Hello again!

Based on my comment on the closed issue here, I think that clean dry run should match the regular clean in terms of logging (on line 90). All of the other logs and their levels are great, just this clean dry run didn't match up.

Thank you!

github-actions[bot] commented 5 months ago

:test_tube: Branch testing instructions

This pull request can be tested locally with the following command:

npm exec --yes -- "github:PhasecoreX/igir#patch-1" [commands..] [options]

Comment generated by the Pull Request Commenter workflow.

emmercm commented 5 months ago

@PhasecoreX DEBUG is the right log level, given the definition of INFO being ROM actions taken (copied, moved, symlinked, deleted, etc.) and DEBUG being ROM actions skipped - and in this case, cleaning was skipped.

PhasecoreX commented 4 months ago

Right, that is the definition of the logging levels, but I think that this clean dry run debug should be an exception. If a user wants to clean files with the clean command, they would see which files are cleaned up at the info level, by definition. If they want to first see which files would be cleaned up, before doing the actual cleaning, they would first run clean in dry run mode. If dry run mode isn't the same logging level as the normal clean mode, they would have to add the dry run argument, and also remember to add another -v of verbosity. Showing debug then shows a bunch of other log lines they are not interested in, especially if their library is large and nothing moved.

In my mind, adding the dry run mode argument should functionally keep the exact same logging levels as clean, but only skip the actual delete action (and mention it's dry mode I suppose). If they add the dry run argument and then the clean log lines disappear because they're in info logging mode, that's functionally the same as not even using the clean command.

codecov[bot] commented 4 months ago

Codecov Report

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

Project coverage is 93.38%. Comparing base (ddaa41c) to head (7f6adce).

Files Patch % Lines
src/modules/directoryCleaner.ts 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #978 +/- ## ======================================= Coverage 93.38% 93.38% ======================================= Files 89 89 Lines 5579 5579 Branches 1299 1299 ======================================= Hits 5210 5210 Misses 351 351 Partials 18 18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

emmercm commented 4 months ago

I don't have as strong of an opinion, so I'm alright with settling on INFO for these.

github-actions[bot] commented 3 months ago

:lock: Inactive pull request lock

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Comment generated by the GitHub Lock Issues workflow.