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

Add dry run option to CLI #123

Merged
merged 9 commits into from
Jul 22, 2023
Merged

Add dry run option to CLI #123

merged 9 commits into from
Jul 22, 2023

Conversation

denil-ct
Copy link
Contributor

@denil-ct denil-ct commented Jul 17, 2023

Motivation

Resolves #27

Modifications

Add a dry run option to CLI --dry-run

Result

If passed, the changes would be printed to terminal, without actually affecting the file system.

Test Plan

Manually tested with various input combinations.

Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! Just requested that we split the output color changes into a separate PR, and one other suggestion.

@denil-ct denil-ct requested a review from czechboy0 July 17, 2023 14:09
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Two more minor requests, thanks.

Sources/swift-openapi-generator/runGenerator.swift Outdated Show resolved Hide resolved
Sources/swift-openapi-generator/runGenerator.swift Outdated Show resolved Hide resolved
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@czechboy0 czechboy0 requested a review from simonjbeaumont July 18, 2023 23:46
Copy link
Contributor

@czechboy0 czechboy0 left a comment

Choose a reason for hiding this comment

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

One last request, otherwise lgtm.

@czechboy0
Copy link
Contributor

@denil-ct Unfortunately #98 touched similar code as your PR, so there are a few conflicts you'll need to resolve before landing. Once that's all green, and #123 (comment) is addressed, we can land it.

denil-ct added 3 commits July 22, 2023 15:03
…rator into dry-run-cli

# Conflicts:
#	Sources/swift-openapi-generator/GenerateCommand.swift
#	Sources/swift-openapi-generator/GenerateOptions+runGenerator.swift
#	Sources/swift-openapi-generator/runGenerator.swift
@czechboy0 czechboy0 enabled auto-merge (squash) July 22, 2023 14:12
@czechboy0 czechboy0 merged commit 7bb32a9 into apple:main Jul 22, 2023
@denil-ct denil-ct deleted the dry-run-cli branch July 22, 2023 14:24
@czechboy0 czechboy0 added the 🔨 semver/patch No public API change. label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dry run option on the CLI
3 participants