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

Further refactoring on CLI #476

Closed
wants to merge 9 commits into from
Closed

Conversation

grodin
Copy link

@grodin grodin commented Jun 10, 2024

This doesn't add any features or fix any bugs, but does change the style option from --dropbox-style to --style=dropbox (and similarly for the other styles) as discussed in #391.

Joseph Cooper added 8 commits June 9, 2024 16:13
Checking whether we have a single filename '-' or multiple files, none
of which are '-' is parsing and should be part of the parsing step
Similarly to 5b97b8e, this logic is part of parsing the CLI args
The tests in MainTest.kt will fail since the JVM is terminated by the
exitProcess() call. In addition, many of the tests want to check the
return code so exiting directly is a bad idea.
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 10, 2024
@grodin
Copy link
Author

grodin commented Jun 10, 2024

I've got a follow on PR ready to go which adds --help. I can happily push it into this branch but I thought I'd keep it separate to start with.

@hick209 hick209 added this to the ktfmt 1.0 milestone Jun 10, 2024
@hick209
Copy link
Contributor

hick209 commented Jun 10, 2024

As this one breaks the current CLI API contract, let's land it last as make it part of the 1.0.0 release.

Do you think we could land the --help one before this here?

@grodin
Copy link
Author

grodin commented Jun 10, 2024

I've reverted the commit that adds --style= since the other stuff is just refactoring, and I'm quite keen to land that refactoring before I do too much more. I'll follow up immediately with --help and move --style= to a separate PR

@grodin grodin mentioned this pull request Jun 10, 2024
@hick209
Copy link
Contributor

hick209 commented Jun 10, 2024

SGTM 😃

@facebook-github-bot
Copy link
Contributor

@hick209 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@hick209 hick209 removed this from the ktfmt 1.0 milestone Jun 12, 2024
facebook-github-bot pushed a commit that referenced this pull request Jun 13, 2024
Summary:
Intended to land on top of #476

Pull Request resolved: #477

Reviewed By: cortinico

Differential Revision: D58448561

Pulled By: hick209

fbshipit-source-id: 28762cf9a9f40c88d981cb493b0c2f8299e9e622
facebook-github-bot pushed a commit that referenced this pull request Jun 14, 2024
Summary: This was missed on the changes from #476, but luckily our internal tests caught it when I tried to sent out a new release.

Reviewed By: strulovich

Differential Revision: D58528471

fbshipit-source-id: ea4824b295f452b1dc0f7d868664e6792ba07c9c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants