Marnen Laibow-Koser marnen

marnen pushed to master at marnen/resume
@marnen
marnen commented on issue stefankroes/ancestry#103
@marnen

Similarly here. Ancestry 2.0.0 works with acts_as_tree, but 2.1.0 doesn't.

marnen commented on issue stefankroes/ancestry#80
@marnen

@stefankroes: All ancestry methods like subtree or descendants are just where clauses. That's irrelevant. Rails eager loading needs an associatio…

marnen commented on issue forabi/coffee-compiler#5
@marnen

So why are you closing this instead of leaving it open for a future maintainer?

@marnen
Bug: parentheses are removed when converting `+` expressions
@marnen
  • @marnen ed56f77
    Use my fork with built Cucumber release.
@marnen
  • @marnen 180a375
    Use my fork with built Cucumber release.
@marnen
  • @marnen dd8f9b8
    Use my fork with built Cucumber release.
@marnen
marnen commented on pull request lhl/chrome-ssb-osx#15
@marnen

Are you planning a release with this anytime soon?

@marnen

You're shadowing the outer temp here. That seems confusing; is the name collision deliberate? Also, temp isn't usually a great name for a variable.…

@marnen

This conditional statement could probably be a method on its own.

@marnen

This is getting way too long. This whole block should probably extracted into at least one new method, probably several.

@marnen

This is probably another good spot for forEach or something similar.

@marnen

Can you do this without magic numbers? The 4500 represents something, and it would be clearer if you put it into a variable with a meaningful name.

@marnen

Notice that you're repeating the args[0][i] != '/' both here and in the next conditional. Can you consolidate those to avoid the repetition. Also, !=

@marnen

These variable names are quite unclear, especially the difference between flag and flag1. Can you come up with clearer names?

@marnen

This is another place where you could probably do window.savedMatricesNotes || window.savedMatricesNotes.push(4) or something.

@marnen

I think this may be another place where Walter stuck you with a bad design. There's no way that this should be a switch statement; instead, each bl…

@marnen

Do you need to be doing this sort of substring comparison, or is there a more structured way that this data could be stored?

@marnen

I realize that much of this is Walter's code, not originally yours, but I'd be very skeptical of a constructor that exposes this many public proper…

@marnen

Why not put the var declaration before the conditional? That way you don't have to repeat it. You also might be able to make this whole thing more …

@marnen

Again, you don't need the != null, and you might get clearer code from reversing the conditions: if (infoDict && typeof(infodict) === 'object') { A…

@marnen

null is falsy in JavaScript, so you can probably just do if (startBlock) and if (note). Also, I think you're missing a space after if here. :)

@marnen

You might consider using forEach, which has been around since ECMAScript 5.

@marnen

This seems like an odd function name, and I'm very wary of a function that has so many arguments. It looks to me like you're only using a few of th…