-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fixing:#655 #690
Fixing:#655 #690
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello again @SatyamMattoo !
Thanks for another contribution.
Have you tested this yourself be running the CLI locally? I think this will behave exactly the same as "remove", and only remove objects and arrays. Most regular log options will still be displayed.
This issue, like many others, is only roughly specced out. It's more of a reminder to me and our product management team about what work needs doing in future. It's a hard place for you to jump into without any context.
If you're keen to make more contributions, next week I can take a look and find something more suitable for you. And make sure sufficient context is set in the actual issue .
Hello @josephjclark, I apologize for the oversight in my testing process. It appears that my tests only covered the cases I introduced, overlooking others. I will rectify this by reviewing and adjusting the changes accordingly. If you believe this issue may be too complex for me, I am more than willing to redirect my efforts to other tasks better suited to me within the organization. Thank you for your understanding and guidance. Best regards |
@SatyamMattoo no need to apologise! Just remember to test the issue before you even start working on it so that you understand what you're trying to do. If you can't reproduce the problem, how would you expect to fix it? It's not so much that the issue is complex, it's more that the solution isn't totally obvious to me. I welcome ideas - it again, you need to understand the problem first. Always feel free to ask questions! |
I believe I have a potential solution to address this issue. A straightforward approach would involve adjusting the sanitization policy to 'silent', thereby allowing the function to return without executing further actions.
Now where we set When configuring the logger as follows:
The resulting output would be:
This behavior appears to match the expected outcome, effectively disabling logging functionality. |
Hi @SatyamMattoo, I don't think this will help. We already have a log level of I don't know if you saw but I left more notes in the original issue. This actually has nothing to do with sanitisation policies. I'll have a look at this issues this morning and see if can find something more suitable for you. |
Hi @SatyamMattoo I've updated the list of Good First Issues - perhaps you can find something in there to work on? I've tried to fill out the requirements but I expect you will have questions - please submit them in the issue |
I will surely check them out. Thank you so much. |
Short Description
Adding silent mode to the SanitizePolicy in order to disable all logs. Created a silent function to return null when silent mode is enabled.
Related issue
Fixes #655
QA Notes
Added tests for the silent mode testing an object and an array.
Checklist before requesting a review