Code review is pointless

>be 5x engineer on team of a bunch of 0.5x engineers
>i'm the architect for a new project
>spend a few days with the initial commits
>writing one internal library from scratch, and modifying an existing open source library to suit our needs
>+2000-400 lines of code in initial commits
>"""coworkers""" have been given the overview and architecture diagram, but they still don't know what's really going on because they didn't write it with me (their choice, I offered to pair program)
>pull requests sit untouched for a week
>after a week of waiting, review is finally in
>"can you rename this parameter to [insert something worse]?"
>"can you change this indentation?" (even though the IDE did the auto indent based on style rules)
>"can you use an enum here instead of an int?" (despite enums being considered bad practice on the platform)
>and other superficial shit that doesn't change any of the core functionality or APIs

I wasted yet another week waiting around for fucking nothing. I would be light years ahead of where I am now if I didn't have to wait for these brainless clowns.

Has ANYONE actually had a positive experience with code reviews? I'm a senior dev. I firmly believe that review shouldn't be necessary for anyone above mid level. Code review has unilaterally been a massive waste of time on every team I've ever been on.

>I don't get to continue working
>Coworkers have to stop working to review my code
>I've written so much by the time they get to it that they can ONLY make superficial style nitpicks. They don't understand anything that I'm doing from a feature or architecture standpoint, so they have to go after whitespace in a build file

Every time.

Pull requests were a mistake. Keeping master pure is a mistake. Every team should do pair programming + extreme programming + tagged releases.

Change my mind. I work 10x faster alone and quality is the same.

Attached: skank with white claw.jpg (1125x1167, 483K)

Other urls found in this thread:

developer.android.com/topic/performance/reduce-apk-size#remove-enums
twitter.com/NSFWRedditImage

if you have software where someone can siphon money out if they can get in malicious code you definitely need code review. For other cases meh. if everyone was at least 2x engineer its useful to learn from each other maybe.

>if everyone was at least 2x engineer its useful to learn from each other maybe.

I agree, but this is covered by pair programming.

My last job was miserable because of no work-life balance. However, my day to day was pair programming with a brilliant 50x engineer (seriously -- we had 60 guys and he was responsible for at least half of the massive codebase). We didn't do reviews because we watched over each other as we wrote the code. We weren't confused by any of our code because we wrote shit together. Neither of us ever had to wait for ridiculous reviews. God I miss that shit.

I feel like I have chains on now.

Attached: fred_brooks_based.png (500x300, 60K)

>2000-400 lines of code

This is the problem, right here. "Code review" doesn't work when you get a lot of code. If you're trying to review that, the only useful thing would be to have a meeting with the reviewers to walk through what's been programmed, the feature and the tests. The meeting should have them asking about things that might not have been tested, but mostly the meeting should be about them learning how to maintain, or possibly add features to what you've written.

The real problem is probably that you have a total shit management team. If they want code review, they should create and assign stories that can be done quickly, are well planned out, structured so they can be merged safely together with what's in production, and have feature flagging that would let you put stuff into master that's not ready yet so that the features wouldn't result in big PRs - but I'm willing to bet that every time this has happened to you, you had a "product manager" in charge of the feature who's never programmed in their lives.

>2000-400 lines of code

I opened the pull requests around +600-100 but kept pushing commits daily until it grew to that. I was still working on the same feature.

This is what always happens

>Open PR with a reasonable amount of code
>Wait for hours
>Get frustrated and keep working
>By the time the idiots look at it, it's too much for their peanut brains to comprehend
>"hurpa durp can you change this capitalization? merge is blocked until you do :>)"

Demand pair programming. It's the quick solution.

I'm not in a position to demand. I'm the architect of this particular project, but I'm not the most senior person on the team.

There's a staffy (one of the peanut brains, only promoted due to years served) and a manager above me. They both demand code review. Oh, and according to them, this is the only way to write software. Every other way is wrong.

Those white claw selzter are pretty good. No added sugar just a fizzy drink with hint of berry.

they don't taste like they have any alcohol. pretty remarkable desu

Programming productivity inversely proportional to the number of programmers involved, that's well known.

Berries taste gross without sugar desu.

>(despite enums being considered bad practice on the platform)
Really? Why?

They taste good for an alcoholic bev(in that there is no negative taste). Also not drinking a disgusting amount of sugar/carbs while knocking a few back is a plus. Friends still think I'm a fag for drinking them but everything else tastes like shit unless it's a chick drink.

There are no enums in the Android framework. In the early Android days, enums carried significant runtime performance overhead compared to integers.

To keep the type safety of enums with the performance of ints, Google provided the IntDef and StringDef annotations, which prevent devs from passing random unallowed integers as IntDef parameters.

The performance issues are no longer so bad with ART + modern harware instead of Dalvik + pajeet hardware. However, enums still significantly impact DEX size. A single enum can add 1.0 - 1.4 KB to your APK. You imagine putting hundreds of enums in the Android framework -- application size would explode.

This is what git is for. Commit locally, submit for CR. Make the rest of your changes in functional commits. Merge each commit as it's approved, and send out the next one. Branch and rebase as you address feedback.

This should let you push more, smaller CRs that your team can address faster, as well as giving you a more manageable commit history. That said, if you're still piling up CRs 3-4 deep, then you or your manager need to make a change. The least you can do is give them good changes to review though, rather than asking them to review your constantly-changing multi-feature multi-thousand-line diffs.

I give them small, easy to read commits. But I'm not going to sit and wait for a week. I keep pushing local changes because if I wait, now my ~6-8 commits will take 6 weeks to get fully reviewed. Doing it in separate PRs is a productivity death sentence. They take multiple days per pull request!

If a PR comes in with me as a reviewer, I get to it within 48 hours. 90% of the time I do it within 24 hours. Considering my diff size is 10x theirs, I'm not sure why they're so "busy".

Code review can be about the power dynamic in playing the corporate structure as much as keeping bad code out. If you want to have more freedom, consider a smaller company or a more senior position.

I work in open-source, where code review is deeply ingrained the "culture". You're at the complete whim of the maintainers, and they probably don't give two fucks about your agenda or schedule, it could take weeks or even months of back and forth before you code is in a state that they're willing to merge. There is no such thing as pair programming memery, it's entirely about convincing them that it's a patch they want to take.

Also review isn't simply a case of ticking something off, but by saying that you've reviewed/approved something, you're publicly putting your own reputation on the line, so people are going to make sure you do shit properly. It's honestly a great way to develop software.

But I've also worked at a place with completely ineffectual "review" practices before, and I can see that frustration. Fucking everything was funnelled through a single "senior" developer, who obviously was always too busy, and who I personally thought was a fucking retard. And when he did get around to it, there was absolutely no eye for quality and some pretty egregious shit got through. A completely ineffective process that just wasted everybody's time.

It has absolutely been about power dynamics and the almighty org chart. These fucking geeks are so petty it's insane. I've never actually seen people sperg out this much about style choices. The least productive members of the team are the biggest offenders. It's really no wonder why they don't actually make anything -- they obsess over minutia instead of actually implementing features.

I expected some performance hit seeing as you're adding more steps to reach the same goal but not that much, the size increase seems pretty absurd too (I'm assuming you mean a single group of enum's not the singular value increasing size by 1.0-1.4kb).

get some automated style checking in place (locally) that covers all code style aspects of your team/project. this should eliminate a large amount of this 'capitalization'/indentation bs, thus either eliminating the need for reviews, or raising the quality of the comments.

Yep, the size cost is untenable

developer.android.com/topic/performance/reduce-apk-size#remove-enums

I have all of that. I use a style checker and a linter. Doesn't stop them from looking at the code via the github site (not in the IDE) and arbitrarily nitpicking regardless. They nitpick stuff that is already formatted according to the style guide.

Right, is there an agile or safe proponent anywhere in your compaby; might be worth going through them and saying extreme programming best practice is not being taken seriously to the detriment of productivity.

get the linter/style checker definition agreed upon by the entire team. better yet, have the big chief write up the style definition/checker. that way, if they comment on your style, while it passes all the check, they also go against their boss, which is less likely to happen.

Is there a desigh doc specifying these little changes, or is it all just made up on thr fly? Firstly, I would make damn sure there was a design doc that made sure there was no question as to the correct formatting of code. Secondly, you seem to be experienced coding, but less experienced in leadership; both are important if you don't want to just be a code monkey forever. Remember that you are a team, and you should function as a team, otherwise the conflicts you're seeing now are only the tip of the iceberg.

how many pajeets do you work with?

Zero. I work with yuropoors

>Has ANYONE actually had a positive experience with code reviews?
Only when I started writing python code with zero experience or knowledge of it. It was helpful at first because I had no idea of what I was doing.
That was the only case though, there was never anyone competent on the team to review my code despite me being junior so it was either dumb shit you described in the OP or no review at all. Mostly dumb questions from clueless lead who wants to stay relevant.
It would be nice if I was the one doing reviews, because I'm about fed up with the bullshit my seniors push, like flooding syslog with "errors" that are not actually errors and deploying it to production.

What does "Nx engineer" mean?
>in b4 /sqt/

Test

based redittor

imagine the smell

>despite enums being considered bad practice on the platform
Are you guys c++98? What's wrong with Class Enums?

Is the problem the code review or the other developers?
In other words, if some 20x engineer wrote 3000 lines of course, could you review it properly?

People smell, user. Grow up.

Just approve and merge your own pull requests without adding reviewers lmao
Or only set as reviewers people who actually know what they're doing

I’m a 100x engineer, ask me anything.

Wtf does 5x engineer mean?

Why?

>>i'm the architect for a new project

Convince who ever it is in charge that you're 5x, ask them to enforce code review for everyone except you, make sure you're one of the reviewers for everyone else's code that way everything passes through you.
That's what I've seen happen, the more senior engineers got a free pass because they were trusted, people new like I was got free mentoring through comments and rejection, the project moved at a steady pace

This is of course if you're not under any Payment Card Industry Data Security Standard or the like.

try throwing their tea in the toilet or something

Attached: 1492445469477.png (328x382, 222K)

this is why you go self employed.

how's it feel not being anywhere close to a 1000x engineer?

I’m fine where I am. Its important to understand what level you’re on. I only know one 1000x engineer and I will never be on his level.

how do I get a girl like this?
what language should I learn to bury my dick balls deep in that thot?

body language

>Dunning-Kruger brainlet working among his peers
>posts shit he faps to in op to show that his IQ is double digit
>complains that his peers are retards

How does it feel to get paid like a 1X engineer?

It means you have 5 eng certifications.

So you can annotate enums with @Intdef and they're converted to strings at compile-time, sounds way better than using cryptic integer values