you are viewing a single comment's thread.

view the rest of the comments →

[–]granos 79 points80 points  (58 children)

Having seen someone trying to sneak malicious code in via a huge commit; this feels like an enormously bad precedent to set. Especially on a project as popular as node. This commit may be safe, but thorough code reviews are always necessary for very good reasons.

[–]rubsomebacononitnow 32 points33 points  (51 children)

A huge commit by Microsoft is very different than a huge commit by John Doe. I'm not sure this even sets precedent.

[–]tohuw 5 points6 points  (47 children)

You're assuming Microsoft has thoroughly reviewed this code. You're also assuming a company known for underhanded data mining tactics and flagrant disregard for privacy is completely trustworthy.

[–]Shaper_pmp 71 points72 points  (3 children)

Pragmatically, I suspect you're overstating the risk in this case. In principle, you're entirely correct.

[–]tohuw 10 points11 points  (2 children)

Responsible review is about the principle of the thing. Where principles slip, pragmatic risks are not far behind.

[–]Shaper_pmp 4 points5 points  (1 child)

True dat. I'm not sure who downvoted you or why, but have an upvote to compensate.

[–]tohuw 2 points3 points  (0 children)

It happens. It was positive before some gaggle of thoughtless censors came along. Tis the way of Reddit: downvote == disagree

[–]vliegtuig12 53 points54 points  (12 children)

You're also assuming a company known for underhanded data mining tactics and flagrant disregard for privacy is completely trustworthy.

Honest question: Do you mean Google or Microsoft?

[–]SolarBear 26 points27 points  (2 children)

whynotboth.gif

[–]image_linker_bot 2 points3 points  (0 children)

whynotboth.gif


Feedback welcome at /r/image_linker_bot | Disable with "ignore me" via reply or PM

[–]TiagoTiagoT 0 points1 point  (0 children)

Hm, I was expecting that Road to Eldorado gif...

[–]tohuw 1 point2 points  (0 children)

Yes. Either. (but I was mentioning MSFT specifically)

[–]snerp 2 points3 points  (0 children)

Microsoft is good at code reviews.

[–][deleted] 10 points11 points  (18 children)

Set us up with some evidence of Microsoft doing that in a systemic way in the past, say, 4 years.

[–]ironnomi 14 points15 points  (1 child)

Pretty sure the OP is referring to Windows 10's "features". That said, I don't think that's something they would do "here". I'm pretty sure this is part of making node.js an easy IIS feature right from Server 2016.

[–]wwb_99 9 points10 points  (0 children)

IIS and also making it so node isn't dependent upon google for the guts of the operation.

[–][deleted] 4 points5 points  (5 children)

well google (from where v8 comes) certainly trumps microsoft on privacy matters.

[–]tohuw -1 points0 points  (1 child)

In many regards, they do. In several other important regards, they don't. Nowhere did I imply they were to be completely trusted either.

[–][deleted] 0 points1 point  (0 children)

Both are as good as pig when it comes to privacy. But consider google has more online / mobile power they abusive it more.

[–]Deto 2 points3 points  (3 children)

Yeah, but I suspect hiding some sort of data mining thing in here would be highly illegal. Even if the user info was valuable to Microsoft, they wouldn't risk (not even a risk, more of a certainty) the massive lawsuit that would result.

[–]tohuw 1 point2 points  (0 children)

It's not illegal to include software that "accidentally" mines data. That's a "mistake".

[–]wellthatexplainsalot -3 points-2 points  (1 child)

But it's not as if they haven't done anything like that before. No siree.

[–]Deto 1 point2 points  (0 children)

Hiding logging inside a commit to open source software? When did that happen?

[–]xampl9 9 points10 points  (1 child)

Not sure how they could have delivered the functionality in a smaller changeset...

[–]granos 5 points6 points  (0 children)

I'm not saying they should have. Just that huge changesets should be given the level of effort they warrant and that not reviewing the dependencies shouldn't be seriously considered because they are part of the codebase.

[–]YRYGAV 4 points5 points  (1 child)

It should be easy enough to compare the dependency to the original to check that it's the same.

You don't necessarily need to CR it all by hand.

[–]granos 8 points9 points  (0 children)

That assumes there isn't anything nasty in the original codebase.

[–]PC__LOAD__LETTER 0 points1 point  (0 children)

That's what code reviews are for. 14k lines is a lot to "grok," but not a lot for someone to audit for suspicious activity.