Conversation
KevinSchildhorn
left a comment
There was a problem hiding this comment.
Could you change it to add a function for public logging, or add a parameter?
We'd like the public logging to be an opt-in so that by default it wont be public.
2f96505 to
648d20f
Compare
|
@KevinSchildhorn I'm not sure that approach really makes sense in the context of Kermit. The reason is because when Users wouldn't expect the behavior of everything-private-by-default. They would expect logs to be public, just as they are on other platforms, and would elide dynamic data that should not be logged before sending it to Kermit. They would need to do this for the other platforms they are running on in any case. Therefore, I think it makes sense to use All that being said, I've done the implementation of opt-in public logging anyway. However, I can remove that second commit, or alternatively reverse the default, if you agree with my argument above. Note also that this change will now conflict with #409. After that PR is merged, I can rebase this one. |
|
Thank you for adding the opt-in for public logging. We understand your argument and the expected behavior, however we don't want to change the default implementation silently at this time. There may be existing users of Kermit that have sensitive data in their logs, and we don't want to change their logs that they expect are private to suddenly be public. |
Sounds good. I see you're already merged the conflicting changes into this branch. Were there any other changes needed here? |
Change the implementation of native method
darwin_log_with_typeto use%{public}srather than%s.Resolves #400