Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[CBR-345] printing log items compatible with current format #3526

Merged
merged 1 commit into from
Sep 5, 2018
Merged

[CBR-345] printing log items compatible with current format #3526

merged 1 commit into from
Sep 5, 2018

Conversation

CodiePP
Copy link
Contributor

@CodiePP CodiePP commented Aug 31, 2018

Description

this change preserves the format of the logging output
when we switch to the new logging library katip

  • this will be compatible with any tools that grep the textual logs

Linked issue

CBR-97 (user story)
CBR-345

Type of change

  • [~] 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🛠 New feature (non-breaking change which adds functionality)
  • [~] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • [~] 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
  • [~] 🔨 New or improved tests for existing code
  • [~] ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

  • I have read the style guide document, and my code follows the code style of this project.
  • [~] If my code deals with exceptions, it follows the guidelines.
  • [~] I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
  • [~] CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

  • [~] I have added tests to cover my changes.
  • All new and existing tests passed.

QA Steps

stack test cardano-sl-util

Screenshots (if available)

grafik

@CodiePP CodiePP added the wip label Aug 31, 2018
@CodiePP CodiePP self-assigned this Aug 31, 2018
@CodiePP CodiePP requested a review from erikd August 31, 2018 08:43
@CodiePP CodiePP removed the wip label Aug 31, 2018
@CodiePP
Copy link
Contributor Author

CodiePP commented Aug 31, 2018

trying hard on CI ..

@CodiePP CodiePP force-pushed the adiemand/CBR-345/compatible-log-entry-format branch 2 times, most recently from a662fc9 to 5740948 Compare September 1, 2018 22:00
erikd
erikd reviewed Sep 2, 2018
View reviewed changes
util/src/Pos/Util/Log/Scribes.hs Outdated
@@ -49,7 +50,7 @@ mkTextFileScribe :: RotationParameters -> Internal.FileDescription -> Bool -> Se
mkTextFileScribe rot fdesc colorize s v = do
mkFileScribe rot fdesc formatter colorize s v
where
formatter :: LogItem a => Handle -> Bool -> Verbosity -> Item a -> IO Int
formatter :: {-LogItem a =>-} Handle -> Bool -> Verbosity -> Item a -> IO Int
Copy link
Member

Choose a reason for hiding this comment

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

If that's not needed, please drop it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleared

erikd
erikd reviewed Sep 2, 2018
View reviewed changes
util/src/Pos/Util/Log/Scribes.hs Outdated
@@ -100,7 +101,7 @@ mkFileScribeH :: Handle -> Bool -> Severity -> Verbosity -> IO Scribe
mkFileScribeH h colorize s v = do
hSetBuffering h LineBuffering
locklocal <- newMVar ()
let logger :: forall a. LogItem a => Item a -> IO ()
let logger :: forall a. {-LogItem a =>-} Item a -> IO ()
Copy link
Member

Choose a reason for hiding this comment

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

Same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleared

@CodiePP CodiePP force-pushed the adiemand/CBR-345/compatible-log-entry-format branch from 5740948 to ceb2c45 Compare September 2, 2018 09:36
@CodiePP
Copy link
Contributor Author

CodiePP commented Sep 2, 2018

'LogItem a' is only needed when we call a function in 'katip' that takes an 'Item a'

@CodiePP CodiePP force-pushed the adiemand/CBR-345/compatible-log-entry-format branch from ceb2c45 to ffdb717 Compare September 3, 2018 15:46
@CodiePP
Copy link
Contributor Author

CodiePP commented Sep 3, 2018

rebased again on develop, so after CI is happy we can merge.

@CodiePP CodiePP force-pushed the adiemand/CBR-345/compatible-log-entry-format branch 2 times, most recently from 7ba1436 to 81be462 Compare September 3, 2018 21:16
@CodiePP CodiePP force-pushed the adiemand/CBR-345/compatible-log-entry-format branch from 81be462 to 71e6ace Compare September 5, 2018 07:28
@CodiePP
Copy link
Contributor Author

CodiePP commented Sep 5, 2018 •
edited
Loading

rebased on develop again, with the change of PR #3537 that lets pass tests.walletIntegration

@CodiePP CodiePP merged commit cc689a6 into develop Sep 5, 2018
@CodiePP CodiePP deleted the adiemand/CBR-345/compatible-log-entry-format branch September 5, 2018 14:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@erikd erikd erikd approved these changes

Assignees

@CodiePP CodiePP

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@CodiePP @erikd