Syntax maintainence #104

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
2 participants

MyGGaN commented Jun 12, 2012

Fixed missing/excessive semicolons, added proper identity operators when missing, unified the mixed used of tabs/spaces (to spaces with 2 spaces as indentation).

@postwait postwait and 1 other commented on an outdated diff Aug 15, 2012

test/test-buffer.js
q.subscribeRaw(function (m) {
var data;
m.addListener('data', function (d) { data = d; });
m.addListener('end', function () {
recvCount++;
m.acknowledge();
+
+ //TODO: This switch should be an if statement
@postwait

postwait Aug 15, 2012

Owner

The switch is fine, remove the TODO.

@MyGGaN

MyGGaN Aug 15, 2012

I removed the TODO. But what's the reason to have a switch-case with one case over a simple if-else statement?

@postwait

postwait Aug 15, 2012

Owner

b/c if it was that important you would have changed two lines of code instead of writing a comment.

The switch is clearer of intention.

@postwait postwait and 1 other commented on an outdated diff Aug 15, 2012

@@ -19,6 +19,7 @@ connection.addListener('ready', function () {
var exchange = connection.exchange('clock', {type: 'fanout'});
var q = connection.queue('my-events-receiver');
+ console.log(q);
@postwait

postwait Aug 15, 2012

Owner

no console.log here please.

@MyGGaN

MyGGaN Aug 15, 2012

Well spotted, sry!

Owner

postwait commented Aug 15, 2012

Now... can you rebase this off current master, it won't merge cleanly.

Owner

postwait commented Mar 1, 2013

Sorry, this just isn't worth it. The code is readable as is and introducing changes that are coding style is disruptive to finding regressions.

postwait closed this Mar 1, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment