-
Notifications
You must be signed in to change notification settings - Fork 119
有关 disable-current-user 的若干优化 #291
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
src/user.js
Outdated
| const AVError = require('./error'); | ||
| const AVRequest = require('./request').request; | ||
|
|
||
| var disableCurrentUserWarning = false; |
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.
var -> let
这个为什么要只打印一次呢?如果我调用一个有问题的函数,那这个函数只输出了一次错误,这个暴露是不是有点奇怪? |
|
因为实际测试的效果就是如果用户的程序中有地方用了 currentUser,将会打印非常大量的警告(可能一个请求会引发好几次警告)。加上这个修改将 warn 换成了 trace(调用栈),所以日志量会更大。 |
比如在我程序中 AV._config.disableCurrentUser = true 的情况下调用了一次 currentUser,应该只有一次警告,调用两次就有两次警告是比较合理的。如果出现多次,那是应该修正,但是目前程序看起来是所有不正确调用,只出现一次警告,就不太合理了。 |
Current coverage is 63.82%
@@ master #291 diff @@
==========================================
Files 27 27
Lines 2896 2905 +9
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 1850 1854 +4
- Misses 1046 1051 +5
Partials 0 0
|
|
我觉得既然是警告的话,只打印一次是合理的;或者就是把这个地方改为一个异常,这样才能迫使用户修改代码。实际上关于警告和异常我一直都在犹豫。 |
|
讨论结论:每次都打印一条 warning(含文档链接),调用栈打印在 debug 里。 |
#251