refactor(CLI): Adapt logWarning to modern logs
#10078
Conversation
| @@ -122,6 +123,16 @@ module.exports = (loadedPlugins, { providerName }) => { | |||
| ).join('\n - ')}\n\n` + | |||
| 'Please report this issue in plugin issue tracker.' | |||
| ); | |||
| log.warning( | |||
There was a problem hiding this comment.
I think it should be still a warning in new logs, I've changed the text slightly
Codecov Report
@@ Coverage Diff @@
## master #10078 +/- ##
==========================================
- Coverage 85.41% 85.38% -0.03%
==========================================
Files 333 333
Lines 13537 13570 +33
==========================================
+ Hits 11562 11587 +25
- Misses 1975 1983 +8
Continue to review full report at Codecov.
|
lib/cli/handle-error.js
Outdated
| @@ -98,6 +98,7 @@ module.exports = async (exception, options = {}) => { | |||
| // It can happen e.g. during "plugin uninstall" command, where "serverless" originally | |||
| // installed a as peer dependency was removed | |||
| logWarning('Could not resolve path to locally installed error handler'); | |||
| log.warning('Could not resolve path to locally installed error handler'); | |||
There was a problem hiding this comment.
As a user what does this mean and what should I do?
I'm asking this because I'm looking if we cannot either clarify this, turn it into an error or silence it to --verbose (if there isn't anything the user can do with the information).
There was a problem hiding this comment.
This is really an edge case that will happen very rarely - turning it into an error is not an option as it's part of the error handler. We can turn it into verbose log here I guess.
As a user, it doesn't really mean much and it's highly unlikely that you will see it, it's more of a guide for debugging of an error so we can also turn it into debug here I think
There was a problem hiding this comment.
Right, so I'd say if users cannot do anything and cannot even understand what that means verbose or debug level is probably best?
There was a problem hiding this comment.
changed to debug 👍
pgrzesik
force-pushed
the
adjust-logwarning-to-modern-logs
branch
from
1eeeb69
to
847e51c
Compare
lib/plugins/deploy.js
Outdated
| this.serverless.processedInput.commands.length === 1 && | ||
| this.serverless.processedInput.commands[0] === 'deploy'; |
There was a problem hiding this comment.
Nit: it'll be more concise to:
this.serverless.processedInput.commands.join(" ") === 'deploy'
pgrzesik
force-pushed
the
adjust-logwarning-to-modern-logs
branch
from
847e51c
to
861c1b3
Compare
Addresses: #9860
Additionally, it adapts
logInfofor completeness, but I believe it will be removed withv3anyway as thePromiseTrackerlogic is only used in old variable resolver.Additionally, it introduces warning for
deploy -falias