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

(#158) Relegate the -lo/-li warning to log-file-only #3167

Merged
merged 2 commits into from
May 24, 2023

Conversation

vexx32
Copy link
Member

@vexx32 vexx32 commented May 24, 2023

Description Of Changes

  • Relegate the warnings for -lo / -li etc to log file only.
  • Update relevant tests

Motivation and Context

After some discussion internally we determined that for folks to have smooth migration paths a log-file-only warning makes the most sense.

Testing

  1. Run choco list -lo
  2. Verify no warning is shown
  3. Check the log file
  4. The logs should contain a warning about the unsupported option
  5. Repeat with -li instead of -lo

Operating Systems Testing

Win11

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #158

After some discussion internally we determined that for folks to have
smooth migration paths a log-file-only warning makes the most sense.
@vexx32 vexx32 requested a review from corbob May 24, 2023 18:19
@vexx32 vexx32 changed the title (#158) Relegate the -lo warning to log-file-only (#158) Relegate the -lo/-li warning to log-file-only May 24, 2023
@vexx32 vexx32 force-pushed the list-lo-warnings branch from 7cc1a7c to 269115e Compare May 24, 2023 18:37
Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

LGTM. Will wait for it to pass builds before merging.

@@ -101,15 +101,27 @@ Describe "choco list" -Tag Chocolatey, ListCommand {

Context "Listing local packages with unsupported argument outputs warning" -ForEach @('-l', '-lo', '--lo', '--local', '--localonly', '--local-only', '--order-by-popularity', '-a', '--all', '--allversions', '--all-versions', '-li', '-il', '-lai', '-lia', '-ali', '-ail', '-ial', '-ila') {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the name of this test change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm, yeah, sure, why not

@vexx32 vexx32 force-pushed the list-lo-warnings branch from 269115e to 74aadea Compare May 24, 2023 19:35
@corbob corbob merged commit 0ad2978 into chocolatey:master May 24, 2023
@vexx32 vexx32 deleted the list-lo-warnings branch May 25, 2023 12:12
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.

3 participants