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

[REFACTOR] Unify case usage, seperators and punctuation for logging #298

Closed
Kavindu-Dodan opened this issue Jan 20, 2023 · 5 comments
Closed
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers Needs Triage This issue needs to be investigated by a maintainer

Comments

@Kavindu-Dodan
Copy link
Contributor

Kavindu-Dodan commented Jan 20, 2023

Requirements

Flagd logs have mixed usage of uppercase and lowercase.

je.Logger.Error(fmt.Sprintf("parseFractionalEvaluationData: %v", err)) vs fs.Logger.Error(fmt.Sprintf("Error fetching after Create notification: %s", err.Error()))

This issue focuses on unifying case usage throughout the source and setting the standards for logging formats.

  • Prefered first letter case - lowercase

    ex:- fs.Logger.Debug("configuration modified")

  • Prefered separator for details - :

    (Note the : usage for for uri detail )
    ex:- Logger.Info(fmt.Sprintf("watching filepath: %s", fs.URI))

  • Prefer log line end punctuation - NONE (i.e - No full stop)

    ex:- fs.Logger.Info("Starting filepath sync notifier")

@Kavindu-Dodan Kavindu-Dodan added enhancement New feature or request Needs Triage This issue needs to be investigated by a maintainer labels Jan 20, 2023
@Kavindu-Dodan Kavindu-Dodan changed the title [FEATURE] Unify case usage, seperators and punctuation for logging [REFACTOR] Unify case usage, seperators and punctuation for logging Jan 20, 2023
@toddbaert toddbaert added good first issue Good for newcomers help wanted Extra attention is needed labels Jan 20, 2023
@mihirm21
Copy link
Member

mihirm21 commented Jan 22, 2023

Hi, can I be assigned this issue or should I directly submit my PR here?

@beeme1mr
Copy link
Member

Sure, thanks @mihirm21!

@beeme1mr beeme1mr removed the help wanted Extra attention is needed label Jan 22, 2023
This was referenced Jan 22, 2023
@mihirm21
Copy link
Member

Could this be merged though DCO not permitting since I am unable to sign off my commit.

@beeme1mr
Copy link
Member

Hey @mihirm21, you should be able to run these commands to sign off on your previous commits.

To add your Signed-off-by line to every commit in this branch:

Please let me know if you run into any issues.

@mihirm21
Copy link
Member

plz check my PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers Needs Triage This issue needs to be investigated by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants