-
Notifications
You must be signed in to change notification settings - Fork 17
Hide kuzzle object from console.log #520
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
Conversation
Codecov Report
@@ Coverage Diff @@
## 7-dev #520 +/- ##
==========================================
- Coverage 95.59% 95.57% -0.03%
==========================================
Files 32 32
Lines 1318 1311 -7
==========================================
- Hits 1260 1253 -7
Misses 58 58
Continue to review full report at Codecov.
|
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.
A couple issues:
- you should mention what versions of kuzzle accept this option
- this looks like a circular dependency in disguise; why do we need to release the support for this option before we release the version of kuzzle accepting it?
I'm still not sure about what Kuzzle version it's gonna be. It's an enhancement since the SDK is already exposed but I still feel like being able to use it with the HotelClerc features is a big improvement and it may deserve the new-feature tag IMHO, what do you think ? About the circular dependency, I have no choice unfortunately because I cannot use the But in this case it's not a big deal since this option is not recognized by Kuzzle for now. |
What do you think about hiding the documentation (you can left it there, but comment it), so that once Kuzzle is released with that feature, you can uncomment the SDK documentation, adding the right Kuzzle version (and a link to the appropriate guide/API doc)? I fear that documenting a feature that cannot be used (at the moment) is not a good idea. |
@scottinet I'm OK with that. |
697f88a
to
d1dbcfe
Compare
@Leodau @scottinet |
src/core/security/Role.js
Outdated
this._kuzzle = kuzzle; | ||
Reflect.defineProperty(this, '_kuzzle', { | ||
value: kuzzle, | ||
writable: true |
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.
Why is this property writable?
What does this PR do?
Use
Reflect
to hide the kuzzle object from logOther changes
delete
keyword and useMap
for RAM caches_id
torealtime.publish