poetry icon indicating copy to clipboard operation
poetry copied to clipboard

mark dry run in Poetry output

Open piotr-kubiak opened this issue 1 year ago • 6 comments

Pull Request Check List

A proposed fix for #9972.

  • [x] Added tests for changed code.

Summary by Sourcery

Tests:

  • Add tests to verify the output of the commands.

piotr-kubiak avatar Jan 06 '25 23:01 piotr-kubiak

Reviewer's Guide by Sourcery

This pull request marks dry runs in Poetry output, addressing issue #9972. It modifies several test files to assert the presence of "Running in DRY RUN mode" in the output when the --dry-run flag is used. It also modifies the version, publish, update, install, and remove commands to print "Running in DRY RUN mode" to the console when the --dry-run flag is used.

Sequence diagram for dry run mode output in Poetry commands

sequenceDiagram
    actor User
    participant CLI
    participant Command
    participant IO

    User->>CLI: Execute command with --dry-run
    CLI->>Command: handle()
    alt dry-run option enabled
        Command->>IO: write_line('Running in DRY RUN mode')
    end
    Command->>IO: write regular command output
    IO-->>User: Display output

Class diagram showing modified Poetry commands

classDiagram
    class IO {
        +write_line(text: str)
    }

    class Command {
        +handle(): int
        +option(name: str): Any
        #_io: IO
    }

    class VersionCommand {
        +handle(): int
    }

    class Publisher {
        +publish()
        #_io: IO
    }

    class Installer {
        +run(): int
        +is_dry_run(): bool
        #_io: IO
    }

    Command <|-- VersionCommand
    Command --> IO
    Publisher --> IO
    Installer --> IO

    note for Command "Modified to show dry-run mode"
    note for Publisher "Added dry-run output"
    note for Installer "Added dry-run output"

File-Level Changes

Change Details Files
Added assertions to test the output of dry run commands.
  • Added assertions to check for the presence of "Running in DRY RUN mode" in the output when the --dry-run flag is used.
  • Modified existing assertions to account for the new output string.
  • Removed unnecessary assertions that are no longer relevant.
tests/console/commands/test_publish.py
tests/console/commands/test_version.py
tests/console/commands/test_install.py
tests/console/commands/test_update.py
tests/console/commands/self/test_remove_plugins.py
Modified commands to print "Running in DRY RUN mode" when the --dry-run flag is used.
  • Added a conditional statement to print "Running in DRY RUN mode" to the console when the --dry-run flag is used.
  • Refactored existing code to accommodate the new output string.
src/poetry/console/commands/version.py
src/poetry/publishing/publisher.py
src/poetry/installation/installer.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in the pull request body to generate a PR summary at any time. You can also use this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

  • Contact our support team for questions or feedback.
  • Visit our documentation for detailed guides and information.
  • Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai[bot] avatar Jan 06 '25 23:01 sourcery-ai[bot]

If I recall correctly, the output is intentionally similar. I don't think there is a need for this change. Happy to hear from others before deciding.

If we do proceed, I do think the message will have to change.

abn avatar Jan 07 '25 11:01 abn

@abn Sorry to hear that. I explained my rationale for this change in the original issue #9972.

piotr-kubiak avatar Jan 07 '25 11:01 piotr-kubiak

I do not have a strong opinion. I see benefits on both sides.

radoering avatar Jan 07 '25 16:01 radoering

I don't mind adding some markers about the simulation mode, but I agree the message should be different.

Secrus avatar Jan 08 '25 12:01 Secrus

I am more than happy to change the message, but not sure what it should be. Here are some examples in other tools:

$ rsync --dry-run -av foo/ bar/
sending incremental file list
./

sent 95 bytes  received 19 bytes  228.00 bytes/sec
total size is 0  speedup is 0.00 (DRY RUN)

$ pip install --dry-run six
Collecting six
  Using cached six-1.17.0-py2.py3-none-any.whl.metadata (1.7 kB)
Using cached six-1.17.0-py2.py3-none-any.whl (11 kB)
Would install six-1.17.0

$ git clean -fd --dry-run
Would remove bar/
Would remove foo/

piotr-kubiak avatar Jan 08 '25 15:01 piotr-kubiak