Skip to content

fix: add check if msg and meta properties are deleted and give priority to log.meta in meta object #14

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

Merged
merged 5 commits into from
Sep 28, 2022

Conversation

kruczyna
Copy link
Collaborator

Added a check to make sure the msg and meta properties are deleted. Did not return as it's not a business critical issue. Also, the coverage dropped to 91%, did not cover those lines with unit tests yet. Is this something desired, though? I am not sure about that TBH 😅
Context:
If we don't delete it will end with a duplicated messge, something like msg: 'I am a message' and message: 'I am a message. If we do not delete the meta property we'll not have more custom meta in the LogEntry.

@metcoder95
Copy link
Owner

I think is not necessary to do such checks but I agree we can do a fast check before deleting the property as can reduce unnecessary burden 👍

About the coverage is ok if it drops but we should always aim to keep it around ~90% 👍

Copy link
Owner

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

Can we add some tests to it?

@kruczyna kruczyna changed the title Add check if msg and meta properties are deleted Add check if msg and meta properties are deleted and give prio to log.meta in meta object Sep 28, 2022
Copy link
Owner

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@metcoder95 metcoder95 changed the title Add check if msg and meta properties are deleted and give prio to log.meta in meta object fix: add check if msg and meta properties are deleted and give priority to log.meta in meta object Sep 28, 2022
@metcoder95 metcoder95 merged commit cd98cbc into metcoder95:main Sep 28, 2022
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.

2 participants