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 kåren misshap from today #133

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Fix kåren misshap from today #133

merged 2 commits into from
Aug 15, 2024

Conversation

The1Penguin
Copy link
Contributor

No description provided.

Copy link

@malmz malmz left a comment

Choose a reason for hiding this comment

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

good shit

@The1Penguin The1Penguin marked this pull request as ready for review August 14, 2024 20:02
Copy link
Contributor

@Rembane Rembane 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 good. I have a small request though: add what Kåren did the commit message and what effect it had on the operation of mat. It's very much optional, but I find it very enlightening to have that kind of documentation in the commit message.

@@ -120,7 +120,7 @@ parse lang =
menuParser :: Value -> Parser Menu
menuParser = withObject "Menu Object" $ \obj ->
Menu
<$> (obj .: "dishType" >>= (.: "name"))
<$> (obj .: "dishType" >>= maybe (pure "Unknown menu") (.: "name"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that Kåren has changed the payload to sometimes not return an object with a "name"-field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This occured once, which was the morning of today, the 14th of august. Later, around ten, the payload was updated and looked as it has done before. This is a precaution if it would happen again.

"parses a blob of JSON without error, but it has an dish without dishType"
( testFun
(Right [ Menu
(T.pack "Unknown menu")
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. I see. Lovely surprise!

Kårres sent a payload which included a dish that didn't mention the
dishType. This would allow such payloads to show up, but listed as
"Unknown Menu", which allows the ones that properly parse to also be
shown along this oddity.
@The1Penguin
Copy link
Contributor Author

This looks good. I have a small request though: add what Kåren did the commit message and what effect it had on the operation of mat. It's very much optional, but I find it very enlightening to have that kind of documentation in the commit message.

Added a comment of what the fix does and why it was needed, kinda

@Rembane
Copy link
Contributor

Rembane commented Aug 14, 2024

@The1Penguin Awesome! Thank you!

Copy link
Contributor

@SwedishSubmarine SwedishSubmarine left a comment

Choose a reason for hiding this comment

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

cute 👍

@The1Penguin The1Penguin merged commit 90e7ffe into main Aug 15, 2024
1 check passed
@The1Penguin The1Penguin deleted the if-null branch August 15, 2024 20:17
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.

4 participants