Skip to content

rocketchat-lib part1#6553

Merged
engelgabriel merged 4 commits into
RocketChat:developfrom
ggazzo:rocketchat-lib
Apr 4, 2017
Merged

rocketchat-lib part1#6553
engelgabriel merged 4 commits into
RocketChat:developfrom
ggazzo:rocketchat-lib

Conversation

@ggazzo

@ggazzo ggazzo commented Mar 31, 2017

Copy link
Copy Markdown
Member

No description provided.

@@ -0,0 +1,19 @@
/* globals LoggerManager */
RocketChat.settings.get('Log_Package', function(key, value) {
return (typeof LoggerManager !== 'undefined') && LoggerManager !== null && (LoggerManager.showPackage = value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplify to LoggerManager && LoggerManager.showPackage = value;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

actually, if the variable LoggerManager have never been declared, It will throw an exception

LoggerManager && LoggerManager.showPackage = value; // Uncaught ReferenceError: LoggerManager is not defined
typeof LoggerManager != 'undefined' && LoggerManager.showPackage = value; // nothing happens

});

RocketChat.settings.get('Log_File', function(key, value) {
return typeof LoggerManager !== 'undefined' && LoggerManager !== null && (LoggerManager.showFileAndLine = value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here


RocketChat.settings.get('Log_Level', function(key, value) {
if (value != null) {
if (typeof LoggerManager !== 'undefined' && LoggerManager !== null) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplify to LoggerManager != null

LoggerManager.logLevel = parseInt(value);
}
Meteor.setTimeout(() => {
return typeof LoggerManager !== 'undefined' && LoggerManager !== null && LoggerManager.enable(true);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here

@@ -0,0 +1,8 @@

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Where is the .coffee file?

@@ -0,0 +1,1006 @@
RocketChat.settings.add('uniqueID', process.env.DEPLOYMENT_ID || Random.id(), {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you keep the comments?

hidden: true
});

RocketChat.settings.addGroup('Accounts', function() {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you keep the comments?

/* globals WebAppInternals*/
RocketChat.settings.onload('CDN_PREFIX', function(key, value) {
if (_.isString(value || false)) {
return typeof WebAppInternals !== 'undefined' && WebAppInternals !== null && WebAppInternals.setBundledJsCssPrefix(value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplify this line


Meteor.startup(function() {
const value = RocketChat.settings.get('CDN_PREFIX');
if (_.isString(value || false)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Any string value will be considered true?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

My wish was testing if the variable is a string or it is not an empty string.

"asda"||false - > "asda"
""||false - > false

because _.isString("") === true

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, but that looks strange and will allow spaces. What about if (_.isString(value) && value.trim()) just to be more clear?

Meteor.startup(function() {
const value = RocketChat.settings.get('CDN_PREFIX');
if (_.isString(value || false)) {
return typeof WebAppInternals !== 'undefined' && WebAppInternals !== null && WebAppInternals.setBundledJsCssPrefix(value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Simplify this line


RocketChat.settings.get('Log_Package', function(key, value) {
return (typeof LoggerManager !== 'undefined') && LoggerManager !== null && (LoggerManager.showPackage = value);
return log(LoggerManager => LoggerManager.showPackage = value);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not LoggerManager && LoggerManager.showPackage = value?

RocketChat.settings.onload('CDN_PREFIX', function(key, value) {
if (_.isString(value || false)) {
return typeof WebAppInternals !== 'undefined' && WebAppInternals !== null && WebAppInternals.setBundledJsCssPrefix(value);
return testWebAppInternals(WebAppInternals => WebAppInternals.setBundledJsCssPrefix(value));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not WebAppInternals && WebAppInternals.setBundledJsCssPrefix(value)?

@rodrigok rodrigok added this to the 0.55.0 milestone Apr 3, 2017
@rodrigok

rodrigok commented Apr 4, 2017

Copy link
Copy Markdown
Member

@ggazzo Can you fix the conflicts?

@ggazzo

ggazzo commented Apr 4, 2017

Copy link
Copy Markdown
Member Author

yes, I can.

@engelgabriel engelgabriel merged commit 413ac7a into RocketChat:develop Apr 4, 2017
@ggazzo ggazzo deleted the rocketchat-lib branch April 4, 2017 18:23
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.

3 participants