Skip to content

Remove unnecessary logging#29

Closed
johanflint wants to merge 1 commit intomasterfrom
fix/remove-logging
Closed

Remove unnecessary logging#29
johanflint wants to merge 1 commit intomasterfrom
fix/remove-logging

Conversation

@johanflint
Copy link
Contributor

Use breakpoints in your browser instead.

Use breakpoints in your browser instead.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Console messages are not visible to the user. I think it's better to either remove the error handler completely (the client will then take care of it) or use mx.ui.error to show a message to the user.

@Andries-Smit
Copy link
Contributor

Or use the "mendix/logger"
that can output the messages in debug mode

@JelteMX JelteMX closed this Jan 4, 2016
@Andries-Smit
Copy link
Contributor

@JelteMX The API does not allow to use the logger the AMD way
The only way is to use the global definded

logger.log("Hello World!")

Would it better to allow it in the api and get the logger in the AMD way?

require("mendix/logger", function(logger){
    logger.log("Hello World!")
 })

@JelteMX
Copy link
Contributor

JelteMX commented Jan 4, 2016

@Andries-Smit I don't know if it's possible get the logger in the AMD way. I'll ask around. If I look at the client API, the logger is actually required as:

define([
    ..., "mendix/logger", ...
], function(..., logger, ...) {

    window.logger = logger;

The logger is always exposed globally. logger.log is not available by the way. For the logger we defined a couple of loglevels:

var ALL = 0, DEBUG = 1, INFO = 2, WARN = 3, ERROR = 4;

The logger methods are basically wrappers for console.

In the AppStoreWidgetBoilerplate (and what we now try to do consistently) is use logger.debug and setting logger.level(logger.DEBUG) if needed. This way a client's console isn't flooded with log messages if it isn't needed

@JelteMX JelteMX deleted the fix/remove-logging branch December 7, 2016 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants