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

Jump to bottom

Re-Implemente subcmd on github.com/urfave/cli/v2 #121

Merged
merged 6 commits into from May 15, 2020
Merged

Re-Implemente subcmd on github.com/urfave/cli/v2 #121

merged 6 commits into from May 15, 2020

Conversation

raa0121
Copy link
Member

@raa0121 raa0121 commented May 14, 2020 •
edited

github.com/urfave/cli/v2 を使って、subcmd を再実装しました。

マジックナンバーで、定数にすべきところも、一旦、そのまま埋め込んであります。

また、 #95 も実装してあります。

kuuote
kuuote reviewed May 14, 2020
View reviewed changes
subcmd/download_emoji.go Outdated Show resolved Hide resolved
kuuote
kuuote approved these changes May 14, 2020
View reviewed changes
Copy link
Member

@kuuote kuuote left a comment

Choose a reason for hiding this comment

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

よさそう

koron
koron requested changes May 14, 2020
View reviewed changes
Copy link
Member

@koron koron left a comment •
edited

Choose a reason for hiding this comment

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

ちょっとこまかいですが、ちゃんとライブラリを使うようにしたいです。

あと指摘は1つ分しかしてませんが、4つのサブコマンド全部も同様に考えてください。

subcmd/convert_exported_logs.go Outdated Show resolved Hide resolved
subcmd/convert_exported_logs.go Outdated Show resolved Hide resolved
subcmd/convert_exported_logs.go Outdated Show resolved Hide resolved
subcmd/subcmd.go Outdated Show resolved Hide resolved
kuuote
kuuote reviewed May 15, 2020
View reviewed changes
subcmd/subcmd.go Outdated Show resolved Hide resolved
@koron
Copy link
Member

koron commented May 15, 2020

site_diff が失敗している(引数が変わったんだからそれはそうなる)ので、
念のため手元で手動で確認します。

@koron
Copy link
Member

koron commented May 15, 2020

いったん origin/master で site_diff して、そのあと urface-cli で site_diff しました。
結果差分なしということで CI / Compare Site の失敗は無視して大丈夫です。

koron
koron approved these changes May 15, 2020
View reviewed changes
Copy link
Member

@koron koron left a comment

Choose a reason for hiding this comment

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

LGTM 👍 ですっ!

@koron koron merged commit 85f68f4 into master May 15, 2020
@koron koron deleted the urfave-cli branch May 15, 2020 16:34
@raa0121 raa0121 mentioned this pull request May 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kuuote kuuote kuuote approved these changes

@koron koron koron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@raa0121 @koron @kuuote