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

Sanitize all categories #103

Merged
merged 2 commits into from
Oct 24, 2020
Merged

Sanitize all categories #103

merged 2 commits into from
Oct 24, 2020

Conversation

gandarez
Copy link
Member

This PR implements the recent changes made to sanitization logic. Now all entity types must be considered for sanitization. One important change is the HIDDEN text will only have extension for FileType.

#98

@gandarez gandarez added the p1 Medium Priority label Oct 23, 2020
Copy link
Contributor

@dron22 dron22 left a comment

Choose a reason for hiding this comment

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

Ok, the amount of tests is a little overwhelming. I think we can make this a little bit more concise:

  1. Let's only test WithSanitization with heartbeat of type file. This test should basically verify, that Sanitize is integrated correctly. No need to test different cases here.
  2. I used a test helper function testHeartbeat() to avoid setting up the heartbeat repeatedly. Let's have 3 of these: testHeartbeatFile, testHeartbeatApp and testHeartbeatDomain.

This way there will be less code. I am a lazy guy 😉

@gandarez gandarez requested a review from dron22 October 23, 2020 21:40
Copy link
Contributor

@dron22 dron22 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 like a nice PR to review!

Can you make sure you perform git rebase -i --autosquash origin/master and force push before merging? These fixup commits should only be there during review. We do not want them in the history...

@gandarez gandarez merged commit 0f0abe9 into master Oct 24, 2020
@gandarez gandarez deleted the feature/sanitize-all-categories branch October 24, 2020 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1 Medium Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants