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

fix(log messages) homogenize the emojis used for user output messages #386

Merged
merged 1 commit into from
Nov 22, 2021

Conversation

dduportal
Copy link
Contributor

It's a "short term" fix to ensure that the log messages have the same behavior and all use the same emoji.
It also improve code readability as it's easier to understand the constant result.ATTENTION than \u26A0.

Please note the following changes:

  • As we use the package name result, when a variable had a conflicting name then it had been changes (3 occurends, 2 are in the AMI package)
  • There was slight differences of emojis (2416/2415 for instance) that have been simplified to all use the same from the result package
  • Sources only have FAILURE or SUCCESS states
  • Same for conditions: only FAILURE or SUCCESS states
  • For targets, FAILURE indicates a real error, while SUCCESS is used when nothing had been changed, and ATTENTION when a change is expected (diff) or applied (apply).

Please note that a long term fix is to implement a custom logger package that would provide explicit method such as ValidationErrorLog(), SuccessErrorLog(), ContentChangedLog(), FailureLog() etc. to move the multiple usages of emojis to a central place, and get rid of the result package.

@dduportal dduportal added the bug Something isn't working label Nov 20, 2021
@dduportal dduportal added this to the 0.15.1 milestone Nov 20, 2021
Signed-off-by: Damien Duportal <damien.duportal@gmail.com>
Copy link
Member

@olblak olblak 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.
Iit goes in the right direction.
The next step is to have a specific package for UI

@olblak olblak merged commit 21b2785 into updatecli:main Nov 22, 2021
@dduportal dduportal deleted the fix/log-messages branch November 22, 2021 16:42
@olblak olblak modified the milestones: 0.15.1, 0.16.0 Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants