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

feat: emoji support for both error and success messages #1046

Merged
merged 1 commit into from
Sep 8, 2021

Conversation

alexal
Copy link
Contributor

@alexal alexal commented Sep 8, 2021

Closes #1025
Closes #1024

Copy link
Contributor

@craicoverflow craicoverflow left a comment

Choose a reason for hiding this comment

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

A couple of nits, but I will resolve them in a micro-PR follow up as I'd love to get this merged and begin using it. Thnk you so much @alexal!

@@ -255,7 +256,7 @@ func runCreate(opts *Options) error {

if !opts.wait {
opts.Logger.Info()
opts.Logger.Info(opts.localizer.MustLocalize("kafka.create.info.successAsync", nameTemplateEntry))
opts.Logger.Info(icon.Success(), opts.localizer.MustLocalize("kafka.create.info.successAsync", nameTemplateEntry))
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't put the icon here, because the Kafka instance is being created, and could fail, so the icon might be a false indicator.

@@ -112,5 +113,5 @@ func initConfig(f *factory.Factory) error {
}

func wrapErrorf(err error, localizer localize.Localizer) error {
return fmt.Errorf("Error: %w. %v", err, localizer.MustLocalize("common.log.error.verboseModeHint"))
return fmt.Errorf("%v %w. %v", icon.Error(), err, localizer.MustLocalize("common.log.error.verboseModeHint"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return fmt.Errorf("%v %w. %v", icon.Error(), err, localizer.MustLocalize("common.log.error.verboseModeHint"))
return fmt.Errorf("%v %w. %v", icon.Error(), err, localizer.MustLocalize("common.log.error.verboseModeHint"))

There should only be one space here

@craicoverflow craicoverflow merged commit e66d76d into redhat-developer:main Sep 8, 2021
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.

Prefix error messages with "❌" Use ✔️ emoji in success messages
2 participants