Skip to content

fix: move alert messages to custom method#461

Merged
wolfenrain merged 1 commit intomainfrom
fix/move-alert-messages-to-custom-method
Jul 15, 2022
Merged

fix: move alert messages to custom method#461
wolfenrain merged 1 commit intomainfrom
fix/move-alert-messages-to-custom-method

Conversation

@wolfenrain
Copy link
Contributor

Description

After talking with @renancaraujo and @felangel we came to the conclusion that we were abusing the alert message, it is meant to indicate something severe or a call to action but we mostly used it for the pretty colors.

Fixing that by moving to a custom message that uses these pretty colors.

Type of Change

  • ✨ New feature (non-breaking change which adds functionality)
  • 🛠️ Bug fix (non-breaking change which fixes an issue)
  • ❌ Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 Code refactor
  • ✅ Build configuration change
  • 📝 Documentation
  • 🗑️ Chore

@wolfenrain wolfenrain requested a review from felangel as a code owner July 15, 2022 10:52
@wolfenrain wolfenrain self-assigned this Jul 15, 2022
/// Extension on the Logger class for custom styled logging.
extension LoggerX on Logger {
/// Log a message in the "created" style of the CLI.
void created(String message) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor but thoughts on calling this something like pretty or emphasis?

Suggested change
void created(String message) {
void emphasis(String message) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought having explicit methods for explict type of logging might make more sense for the CLI itself?

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m fine either way just was thinking about if we have another non created message with the same styling

@wolfenrain wolfenrain merged commit ec31ed9 into main Jul 15, 2022
@wolfenrain wolfenrain deleted the fix/move-alert-messages-to-custom-method branch July 15, 2022 18:03
@felangel felangel mentioned this pull request Jul 19, 2022
7 tasks
ahsanf pushed a commit to Arkabyte-Teknologi/very_good_cli that referenced this pull request Aug 12, 2025
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.

3 participants