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

Katip + Aeson 2.0.0.0 Compatibility #131

Merged
merged 2 commits into from
Nov 5, 2021

Conversation

tbidne
Copy link
Contributor

@tbidne tbidne commented Nov 2, 2021

Background

The motivation for this PR is #128, to add aeson-2 compatibility. To sum up, the plan was:

  1. Add individual stack.yamls to each sub-project.

For the projects that can be upgraded (katip, katip-datadog, katip-logzio):

  1. Add CPP for aeson 1/2 compatibility.
  2. Pin to aeson-2.0.1.0.

Changes

This PR completes 1 for all projects and 2+3 for katip.

I have 2+3 for the other projects implemented on another branch (you can see the diff here) because I figured it might make sense to merge them separately since katip-datadog and katip-logzio will have to pin the new katip-0.8.7.0 too.

The changes are pretty mechanical, simply (un)wrapping Key to Text. The relevant new aeson modules are Data.Aeson.KeyMap and Data.Aeson.Key.

Tests

You'll notice I had to add new golden files for the tests (see katip/test/Katip/Tests/Scribes/Handle.hs). diff Handle-text.golden Handle-text-aeson2.golden highlights the last two lines. Note that the order of the last few keys (sub, text, num, float) has changed:

< [2016-06-12 12:34:56][foo][Info][example][PID 7331][ThreadId 1337][sub:null][text:][num:0][float:0] message
< [2016-06-12 12:34:56][foo][Info][example][PID 7331][ThreadId 1337][sub.note.deep:some note][text:note][num:10][float:5.5] message
---
> [2016-06-12 12:34:56][foo][Info][example][PID 7331][ThreadId 1337][float:0][num:0][sub:null][text:] message
> [2016-06-12 12:34:56][foo][Info][example][PID 7331][ThreadId 1337][float:5.5][num:10][sub.note.deep:some note][text:note] message

I'm not very familiar with how katip orders these last few keys, but I'm guessing this ordering implicitly comes from aeson, hence the change. Let me know if this is a problem.

The json file, Handle-text.golden, also had the order changed, but the json is structurally identical. I whipped up this gist you can use to verify they're the same.


Let me know if you have any questions, requests, etc. Thanks!

Copy link
Collaborator

@MichaelXavier MichaelXavier left a comment

Choose a reason for hiding this comment

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

This looks great! Thank you!

@MichaelXavier MichaelXavier merged commit 62d2f9b into Soostone:master Nov 5, 2021
@tbidne
Copy link
Contributor Author

tbidne commented Nov 7, 2021

Excellent, thank you for the quick turn-around! Would you like a second PR for katip-datadog and katip-logzio?

@MichaelXavier
Copy link
Collaborator

Sure, if you're feeling up to it!

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.

None yet

2 participants