Skip to content

Support disable_notify and message options in add-devices command#305

Merged
anoworl merged 5 commits intomasterfrom
work/support_disable_notify_and_message_options_in_add_devices
Nov 1, 2022
Merged

Support disable_notify and message options in add-devices command#305
anoworl merged 5 commits intomasterfrom
work/support_disable_notify_and_message_options_in_add_devices

Conversation

@anoworl
Copy link
Copy Markdown
Contributor

@anoworl anoworl commented Oct 31, 2022

In addition to dg deploy command, I supported disable_notify and message options in dg add-devices command.

Checklist

  • dg add-devices --disable_notify
    • No email sent
  • dg add-devices --message hogehoge
    • Release note changed
  • dg add-devices -h

@anoworl anoworl marked this pull request as ready for review October 31, 2022 16:10
@anoworl anoworl changed the title Support disable_notify and message options in add-devices Support disable_notify and message options in add-devices command Oct 31, 2022
@jmatsu jmatsu self-requested a review November 1, 2022 02:00
Comment thread lib/deploygate/command_builder.rb Outdated
command ADD_DEVICES do |c|
c.syntax = 'dg add-devices'
c.description = I18n.t('command_builder.add_devices.description')
c.option '--message STRING', String, I18n.t('command_builder.add_devices.message')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks better for me to use command_builder.deploy.message to represent reuse. What do you think? @anoworl

ref: https://github.com/DeployGate/deploygate-cli/pull/305/files#diff-79f3cff459436871e580bb0a25dc3de5b4c3830f2bb92915d7d811ad65574d85R94

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you. True. I will change it.

Copy link
Copy Markdown
Contributor Author

@anoworl anoworl Nov 1, 2022

Choose a reason for hiding this comment

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

I did! 21cd279

And I checked "dg add-devices -h" outputs.

Comment thread lib/deploygate/command_builder.rb Outdated
c.option '--distribution-key STRING', String, I18n.t('command_builder.add_devices.distribution_key')
c.option '--configuration STRING', String, I18n.t('command_builder.deploy.configuration')
c.option '--server', I18n.t('command_builder.add_devices.server.description')
c.option '--disable_notify', I18n.t('command_builder.add_devices.disable_notify')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ditto

Copy link
Copy Markdown
Contributor Author

@anoworl anoworl left a comment

Choose a reason for hiding this comment

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

Thank you! I reflected and replied the reviews.

@anoworl anoworl requested a review from jmatsu November 1, 2022 02:16
Copy link
Copy Markdown
Contributor

@jmatsu jmatsu left a comment

Choose a reason for hiding this comment

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

LGTM

c.description = I18n.t('command_builder.add_devices.description')
c.option '--user STRING', String, I18n.t('command_builder.add_devices.user')
c.option '--message STRING', String, I18n.t('command_builder.deploy.message')
c.option '--user STRING', String, I18n.t('command_builder.deploy.user')
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great! 💯

@anoworl
Copy link
Copy Markdown
Contributor Author

anoworl commented Nov 1, 2022

Thank you!

@anoworl anoworl merged commit f17260a into master Nov 1, 2022
@anoworl anoworl deleted the work/support_disable_notify_and_message_options_in_add_devices branch November 1, 2022 02:28
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.

2 participants