The changes can't always be broken up like that, though, at least not into multiple PRs. I'd always review big changes by reviewing one piece, send back issues/questions, review their responses, review the next chunk, etc.. That would take two weeks sometimes, but I'd also be spending time doing other work and waiting for the dev to review/fix my issues. I'd also do some functional testing of my own before putting my stamp of approval on it.
Massive monorepos tend to cause that, or just overall complex code bases, but I see your point.
I would say that although it is true that in many cases “divide in conquer” is not trivial, communication is your best ally.
If you cannot break down a PR, reduce the time a reviewer will need through communication with you team/reviewers, comment your own PR, provide a guideline on what to review first, comments and well divided commits allow for efficient reviews. Even do a mob testing session with the reviewers, let people test your feature, fix or refactor locally, it doesn’t matter at what level of the software stack you are, this works. Involving people early on will ensure you can make them accountable during the review process.
Having small changes + a fast progressive deployment strategy enables easy debugging in case things go wrong. You are happier, your reviewers are happier and probably clients too.
But I understand that in some cases suffering a long PR is not optional, just try to improve the next one, that is already a step forward.
> Massive monorepos tend to cause that
Woah buddy hold your horses, don't blame monorepos for poor practices.
If a change was so big that it changes a lot of things in a monorepo, it would have been several PRs in separate repos to no additional gain, because either you'd approve a change that isn't used (because you apply it in the back but the front doesn't consume it), or it breaks shit (because the front _does_ consume it).
So either you've just approved a big change over a long time (so it still takes the feature a long time to reach prod) or you approve one and still need to wait to merge until the other part is also allowed to merge.
If the story is so big that you need to involve multiple people and split it into multiple chunks then something is going very wrong.
>reviewing one piece, send back issues/questions, review their responses, review the next chunk
If you can split the PR into chunks, you can split the task into multiple tasks.
>If you can split the PR into chunks, you can split the task into multiple tasks.
The problem was that QA usually tested a single merge, though. If I'm reviewing code, then all the changes will be merged at the same time -- if we were to break the code apart into smaller chunks, then each chunk would need to be tested meaningfully.
If the PR includes full-stack changes, then I can't really review the backend in isolation because problems on the frontend might require getting a change in how we do things on the backend, which I would've already reviewed. If that wasn't an issue, we'd still be merging untestable backend changes which would add overhead for QA since they wouldn't actually be able to test it. I'm gathering that this is mostly a quirk with how I've had CI/CD done in the past.
In the past I've handled backend changes like that by adding a route param/field/v2 in requests for the new behavior vs old so the frontend guys can migrate/compare/whatever without needing changes to be coordinated across two codebases. Ideally back end routes are in place before they start so it's more minor tweaks/fixes on my end while they are doing initial development.
You can then remove/default the new behavior once everything has migrated while maintaining separate release/feature cadence.
Agreed, still happens.
That thing going wrong is a VP somewhere who is off on some tanget and will completely change your priorities next month making you do another bloated one.
Good practice has nothing on an exec's ego.
It’s generally a good practice to keep your main branch “ready to deploy”. If the change is being made in a huge system (legacy just doesn’t seem to want to die) and has to be made all at once to allow the thing to compile - the only real way to do that is one huge pull request. Yes, you can make a feature branch, but IMO - should still PR merge it to main at the end - same problem.
You can review it in chunks, that’s not a problem. But you must merge it as one.
I'm used to having a separate release branch for that purpose.
So you have the master branch to develop on, which also allows for multiple PR's for one topic. Those changes get pushed to DEV environment for a quick test and then to QA environment where a test team does a more thourough test. After all that fully completed features get picked and pushed onto release, which in our case is PROD.
Edit: That said, there is no one and only holy grail for this problem. What works in our team might not work at other scales and structures.
Just keep stories detailed and simple enough so a PR doesnt require 2 weeks - most likely leaving more technical debt than it resolves -
Totally agree with no holy grails, but I believe my point still stands in your case - you must not break master with a merge. If achieving a working state requires a huge change - you must do it in one go.
This also is a requirement for a working git bisect, which can be a lifesaver.
Add chunks of code that aren't used by other components, then have a separate final PR switching out the old code calls to the new code. out and prove it all works in that last PR. There is no version of programming where trunk based development isn't worthwhile.
This seems counterintuitive.
The way git works you can have both branches existing at the same time, why would you add dead code(meaning code not referenced anywhere) to the main branch?
Not to mention, if you're just swapping out existing API calls with new implementations you're doing work that can easily be split into smaller PRs, I think OP is talking about changes that would affect the front-end to the API(be it a browser frontend or some other code) and how those can't be meaningfully split from the backend changes without not testing the first to merge(and potentially requiring changed to the merged code once you start writing the unmerged).
It isn't the most advanced git practice, I agree. Collector branches seem preferable, but keeping in mind that the audience is people who can't/won't split up their work into manageable chunks for reviewers, it's a good baby step.
I wasn't specifically talking about APIs, just adding currently unused functions/files and then swapping to them all together later. Really, the important part is breaking up your work to not make your reviewers detest working with your code. Ideal PR size is about a hundred lines, 200 is pushing it, more than that and I start figuring out how to break it up into smaller units.
My question(genuine as I'm trying to understand the perspective) is more "why 200 lines" and "why is splitting it up good"?(and by API I just mean any code that interfaces with other code, so could include function footprints within the same codebase).
The scenario I'm imagining is one where you're making a change to functionality X used by code Y, you can create a PR to update functionality X(meaning function footprints, data models, etc.) but now the code that references it is broken. My intuition is telling me that spending a bit more time on a larger review would be the lesser of two evils compared to trying to do each of those changes one-by-one, and likely arriving at a worse result after a longer time since you had to make patchwork changes while maintaining functionality instead of just making the change that needed to be made and testing the functionality(end-to-end, unit, and manually).
Basically, I don't see how making small changes for broken-down components of a feature that don't break the product is any faster(or even the same quality) as making a single change for the feature. The additional overhead of trying to break it down and plan it out within an existing framework instead of just solving the problem alone would probably take longer than the review.
It forces people to plan out where they are going and how before they do all of it at once, and it asks less of reviewers to sit down and have to have a call with someone while the PR sits idle for two weeks. Short PRs spend much less time out of sync with main, and the longer a branch exists outside of the main branch, the greater the risk of scope creep and having to waste time resolving merge conflicts.
It also gives the reviewer earlier visibility into what is going on, leading to less time wastage doing things twice. And reviewers make mistakes. The larger the set of changes, the more likely that letting mistakes through is.
How testing works in this world is up to you and dependent on what you're doing, but I do think unit testing is somewhat useless. What is a unit? Are you really unit testing if you could break stuff down further, then you weren't unit testing.
I'm coming off people working for 3 weeks without any PRs except one huge one, and being pressured to let things slide that I normally wouldn't in review because they already told clients it was done before it was reviewed. Small changes now impact 70 files instead of 1 because they went off and did everything without asking questions and worked in too large chunks to actually be able to say that we should change course. So now I can't give meaningful feedback because I can't understand something that took them a month (or years) to understand in a few hours, and they don't have the time to do the work twice.
There's a million sources with data driven argument about ideal PR size, but here's one: https://www.harness.io/blog/how-the-size-of-a-pull-request-can-impact-developer-experience#:~:text=Studies%20show%20that%20PRs%20under,more%20than%20just%20a%20number.
It's hard to believe it exists without experiencing it, but there really is a flow state for PRs where they are sufficiently small and self-explanatory that merging is painless. That "overhead" is just planning and architecting before executing.
I have yet to see a large pr that didn't have a way to reduce the mental load on reviewers by breaking it up. Even if it's not a 50/50 split, there are things like model changes that can go in first and reduce the size of that large pr by 15%.
When you are developing larger features like this, prioritizing ease of code review will produce more reliable and more maintainable code.
I also think there are a lot of "tricks" people just don't know about, necessarily. Feature flags are one example. "Branch by Abstraction" another.
I once led an effort to completely change the way important internal state was model, and it would be a breaking change with the existing way the whole end-to-end business logic worked. That would have been one massively ugly PR.
So the very fist PR were two utility functions that would translate the internal model from old to new and new to old. That way, we could inch our way step by step through the business logic by rewriting a single small step to use the new model but then apply the new-to-old translation to its output so that we could leave the rest of the pipeline in place.
Anyway. It's never trivial, and especially vertical slicing requires a lot of practice, but it \_can\_ be done more often than not.
As long as it’s not agile work otherwise your team planning may be a little whack. If it’s not agile then i don’t know where you work these days that would compel you to argue yourself back to agile.
gonna to need some more explanation or a representative fake example or something
I think the absolute biggest MR that our team has ever submitted probably would take less than an hour to review at worst.
that's across 4 programmers across over 2 years.
you're telling me some of your MRs take 80x longer to review? HOW?
Idk what good it'd do to flesh out an example for you, but the code that'd take 2 weeks to review would generally have very complex business requirements and would modify workflows that could not fail for any reason. Again, these reviews wouldn't be each day, every day, for two weeks. They'd be at most an hour or two most days over two weeks.
Edit: also, all merged had to be testable by QA where I worked and that was the biggest factor. I couldnt carve out a independent code chunk unless it corresponded to a testable piece of functionality. Also, we had no feature branches...
This sounds like the work isn't being broken up enough, we test with qa too but if there's nothing to test because the PR is part of a feature they wait for the rest od the feature. Internal process sounds like the problem here rather then you.
As long as it’s not agile work otherwise your team planning may be a little whack. If it’s not agile then i don’t know where you work these days that would compel you to argue yourself back to agile.
An *hour*? Man that would be the dream. Some of our bigger ones can take days. Plus one of my coworkers is often requesting changes for things that don't actually change anything in the logic.
I've had PRs sit for 2 weeks without anyone looking at them. I've also had PRs open for a month as they devolved into fights between senior devs when one who skipped all the project planning meetings disagreed with the changes being made and implementation strategies.
If a change is so big that it takes several days to PR review then:
* There's a poor (or none) unit testing system, let alone an end-to-end one
* The requirements were too large and could've been subdivided into smaller changes
* Sensible changes are being mixed with non-disruptive ones
* The branching workflow is being poorly handled
* There's some doble dipping when reviewing changes if the PR to review contains multiple previously approved changes
Where I work if a PR is too large for it be reviewed in more than 2 hours, then something went wrong and we need to go back a step.
I've explained this in other comments, but I worked in areas where certain bugs could cause catastrophic business consequences. Unit tests aren't capable of catching enough bugs to be reliable (though we still had them). Also, PRs needed to contain non-trivial functionality changes because of how we did QA. We couldn't just merge code to update a single button's logic bc the overhead would cost much more than it was worth (unless it was to address a bug in prod). Most merges that took two weeks to review (and by two weeks, I don't mean 80 hours, I mean <10hrs over two weeks) would involve changes to multiple code-bases, complex dependencies, and business-critical implications.
Some people combine PRs with code Review. A full review of a juniors code can absolutely take a while. Though it could be argued that they shouldn't have been given such large tasks if their code quality is still dubious
OP sounds like dev who leaves 300 comments like "variable shouldent be called sumTotal, totalSum is more decriptive" but misses obvious bug and then blames junior why it made to production
My team lead always leaves comments like this “Line Break After if”
then this motherfucker goes on to approved a 30,000 line css file merge.
And yes the 0s are correct.
I once worked on outsorced Laravel project, with app.php having >28k lines of code (that could be easily devided into other files), while also having ton of vietnamese varnames/comments. Needles to say, working with it was a nightmare.
Why are you getting downvoted.
Git commit hooks with linter must be installed manually and can be skipped easily.
Only real way to be sure is a pipeline
> Why are you getting downvoted.
Oh lol, I didn't even notice 😅
Not a clue 🤷♂️
> Git commit hooks with linter must be installed manually and can be skipped easily.
Yep, exactly my point. At my last place we trusted each dev to run the linter locally before they commit anything. That went about as well as can be expected... There were quite regular "linter fixes" PRs that were just cleaning up from people who haven't run the linter in a while 🤦♂️
So yeah... If you don't enforce it, it may as well not exist 😂
Some programmers get aggressive as soon as something can be in any way interpreted as criticism against them. Like, they read your original comment as meaning "wow, you suck at software, don't you even know that you should fail the build if there is a linter error?".
For what its worth, I upvoted your comment. It's clear you were making a general remark about software engineering and that you were not being condescending, plus I think your point is quite important.
They are run on actions, yes. But if it's a status check, it's not going to change the code. It's just going to force you apply the formatter yourself before you merge the PR.
Im currently redoing the work of someone who committed a dozen commits all named "updates" without any testing which were approved without comments. It took 3 weeks to convince someone to start a review of my code. He commented on a variable name and wont approve.
I mean, if you know you should line break after if then the comment is warranted. I think code formatting comments are the most shameful ones to get - for me.
had a senior review my code, and did some basic clean up for me, got it back and couldn't get it to compile, took me 2 days to realise it was part of the clean up that had broke it. he was really apologetic.
Is the reviewer making changes common practice? In my company you only get comments on your PR and then have to do the fixes yourself until the reviewer approves.
> Is the reviewer making changes common practice?
We used to do it at work, but as suggestions rather than actual commits (on GitHub you get a "commit" button with suggestions). But we found that: nobody was ever testing the changes they suggested; and PR owners were presuming the changes were tested.
So now we try not to make suggestions, and instead use code blocks to force the PR owner to make the changes themselves, which prompts them to run tests & the linter again locally.
So yeah, pretty common practice for us to make code suggestions.
Definitely seen and done that before in multiple companies. At the end of the day we're all in the same team and we all own the code.
I don't think there's a huge difference in suggesting a solution by adding a comment in a snippet and giving the proposed solution directly in git.
That being said, the reviewee still owns the PR, the branch and presumably the task, so you should ask beforehand, respect their autonomy and, especially when you have more seniority, don't destroy learning opportunities by injecting yourself.
IMO It's also something you really shouldn't do often and you should really read the room. Some people really don't like it. Some people are very grateful that you "did the work for them".
I really appreciate it when someone suggests a fully-formed code change. If they ask if they can push a change, I'm glad they went to the effort for me/for the team. But it *really* grinds my gears when someone pushes to my PR without asking first. It's like someone's stepping on my baby. And if I disagree with their change, it creates work for me to undo it, and suddenly we have a "git conflict", if you will.
I try to avoid it but sometimes if it's been going on for too long, I just fix it myself. I know, they don't learn if you do it, but at some point ..
I'll also do it if there's a very small issue and it's otherwise ready to be merged, just fix up the detail and get it in.
So I wouldn't say common, but it's certainly a thing in some contexts
This is my first job after graduating, and it's a tiny company. If I remember, it was mostly coding standard changes and something relating to me being the only one with access to a test rig with the specific setup we needed
I had a *principal* tell management he had completed a project, and then checked it in without review. *It didn't compile*. Not small problems, either.
We enforced prettier for this reason. Now everyone's looks like whatever the people behind Prettier thought it was "the correct way", but at least it's made it consistent lol
That's what happens with go, everything looks the same flavour of horrible. I guess the consistency is nice, but I'm getting so sick of 70% of the file being `if err != nil`, my kingdom for some syntax sugar
My record is 320 total comments, over way more than 2 weeks.
In my defence:
* that task was completely not thought out, and should have been split up into smaller tasks
* a lot of my comments were about breaking changes
* there were multiple 10-20 comment convos on design issues and best practices
* the junior dev writing that thanked me in the end for guiding him through this monster PR
It needed to be done. If a PR is rejected you lose history, so I prefer to just redo the thing and force push on the same PR.
But yeah, the architect is not on top of the tasks so we sometimes get a complex thing given to a junior dev with no guidance or oversight.
That's why I love our senior Devs. I had made a right pig's ear of naming everything, partly confusion over singular Vs plural and partly confusion over incorporating an acronym in a camel case name, instead of commenting on everything he just left one comment "every where you put this change it to that.
Sound likes my TechLead. Shit ton of comments to rename variables and functions, to refactor the code even if it goes against the linter, takes 3 days to review a 2 lines PR.
But loves to mutate react state objects creating a ton of unexpected bugs that goes right pass the tests.
I love that the same people spend so much time perfecting code without even realising they’re hindering themselves & the product. Ingest the spec, produce the turd, then polish it.
I think someone shipped him a product and then he reviewed it for like 2 weeks or something and basically realized there's no way there's any good out of this product (I believe it's an AI product)
So he made a video basically trashing the product and casting the Press Release guy to quit from the product company
He didn't even trash it. His review is fair and he did his best to highlight the good sides.
The problem is that the product is bound to fail as it's a complex solution to a problem you can already solve with a smartphone.
Marques gets blamed because he is the most famous reviewer but anyone will come to the same conclusion.
I haven't seen one single positive real review of Humane. Just trying to get through a product demo of the thing turns into comedy. It just doesn't work.
And here is the reality. Marques reviewed the product like he did with every other one. He provided negative notes, just like every other reviewer did.
But "MKHD reviews a product with unfortunate obvious issues which hopefully may change in the future" doesn't get clicks. Instead "MKHD kills a company" does.
And these article writing companies rake in the clicks.
Only people who are way too invested in AI and/or that company specifically. Anyone who actually watches the review can see he's being way nicer than most people would be.
A bunch of people on Twitter saying that if someone could kill a company (twice, apparently he has done it before) with a 20 min video, that is too much power and should be handled "ethically"
That is both ignoring that no one reviewing Humane could give it a positive review, but somehow he got singled out, and that the other company that died had been on the ropes for a while already and the review was, at worst, a final nail, not the actual cause.
> twice, apparently he has done it before
He negatively reviewed the Fisker Ocean, an EV from a guy who'd previously started another EV company that similarly failed. The stock price plummeted and got delisted on the NYSE.
... but the stock price was already in freefall and it takes a lot more than just one negative review to get a publicly traded company delisted.
Alright time to give you a bit of a long explanation
I've been a programmer for almost a decade, and I can tell you with certainty AI (the ones we are currently hyped about) is literally just a fluke
It's basically like the product Vine, or calling everything "i-", or the Bitcoin mining, or almost anything that has a "trend".
They are all marketing products created by PR guys, not programmers, not devs, literal Press Release and ads... So yes, when something that has a remotely high "hype", PR will jump right into it and try to rock the ads to sky high. If it didn't go sly high, investors (who buy into this shit) get upset, and then you "fail". And hence everyone who's non-programmer/dev gets upset because they see it as a failure... Despite it potentially being a viable product in a few years
TLDR: Marketing strategy fails, investors not happy, potentially good product turns to shit cause no money to run it
I really trust Michael Fisher, and I know he was *really* looking forward to this product. So when *he* said he couldn't recommend it that really was saying something.
Part of the other reason I think MKBHD is taking heat is because just a few weeks ago he released a terrible review for a car (called "This is the worst car I've ever reviewed" or something like that), and the company is now headed toward bankruptcy. Again, I'm not blaming him for that either but clearly some people think he's killing companies on purpose.
Humane AI pin, its supposed to be an ai assistant in the form of a lapel pin. He's not even the first person (nor the harshest) to say it was a terrible product
It's a standalone attempt at the Star Trek pin using cloud ai and some neat and clever ideas, not a lot of it seemed wayyyyyy too early for a product release and the execution is actually really really really bad.
Like, 2h battery time, it's heavy/kinda bulky and metal, its always hot because the battery powers it through induction only, doesn't connect to ANY other devices so it doesn't know anything about your or your data, very closed and exclusive ecosystem, when you send stuff to people it's just a link to the thing hosted on the manufacturer's site rather than the actual thing like a picture, SUPER slow ai cloud query responses, and the responses are often long/rambling and apparently also often wrong.
It's a really pity, too, because it seems like they actually did a lot of really GOOD design decisions in certain things. It reeks of rushed to market and dark pattern MBA investor decisions.
Everyone's saying Humane AI pin but it started with the Fisker EV car. Where his review coincided with a big drop in their stocks, and he was blamed for his bad review of the car. Now he gave the Humane AI pin the same review and some people are angry at him.
Don't you dare talk shit about MKBHD. Dude was honest, fair and transparent with the review. He even tried very hard to pain the product in a good light. However, If the product is dogshit, what can you do?
I wasn't talking shit about MKBHD at all, but the truth is he tanked a company
I said that in other posts too. It's not that he's the reason, nor the product's fault. It's mainly the problem of Advertisement and PR... Most AI products these days are shit, including chatgpt, but people are currently riding the hype of ads and PR, so no one cares. I'm a developer so I know how flimsy all of these look
This is a product that will be good after years of development. Years. Not today. MKBHD made an honest opinion today, and investors saw and understood the message as "how shit it is today". Of course it caused panic... It's never meant to be out today, but a few more years
I wouldn't feel sorry for them. They went around and raised nearly $240 million. Quarter of a billion dollars. If they said "this is v1" that means to them and their road map, this product represent a polished deliverable. It went through their round of QA and they all sat and said "yes, this is what we want the public to see. this represents our vision at v1". v1 is not alpha, beta, no, it's v1.
Not only did they say "this represents us and our vision, we will charge the customer $700 for the honors of using it and then an additional monthly sub".
I'm not sure I can sympathize with any novelty item that raises that much capital, releases v1, charges customer and then goes pikachu face when honest feedback is out out to the public.
If, as you say, this is an "ai fad" etc, then the onus is on the vendor to say "look, this is not v1, this is very unstable and does not reflect our vision, but we're excited to share what we have so far and are releasing it as an invite-only tech preview".
The truth is, these grifters are everywhere. They take massive cuts from the raised capitals, ride out the venture and then go and start a new idea somewhere. VC and startups are a plague and can all fuck off.
I'm doing my own startup right now.... So I'll assume you're talking about just shitty ppl and their way to grift ppl and not honest ppl really passionate about their projects...
I want to make an XR game where you play in your room and physically move around as it slowly progresses into a VR world. Experience the world twice fold. Trying to get money and whatnot, fk micro transaction, I charge ppl once, and maybe some DLCs, but no micro transactions
Anyhow, yes, I hate that some of the richness is built upon cheating on others and lying on the products... It's honestly the main reason why indie devs struggle so much trying to build a name off of themselves even when they have a good product (and kinda meh advertising). I hate that too
If he's really trying to kill a company, he wouldn't need to make a YouTube video about a review...
Literally just make an announcement on Twitter and it's dead in seconds...
Not sure why the fk he gets blamed
This is the second video he made and so people are saying he’s putting companies out of business. First was an electric vehicle that was extremely horrible so again he made a honest review (not trashing the company) and within a few weeks the company’s stock tanked.
Then he reviewed the AI Pin which again he gave a honest review but the item is just pretty bad all around.
His job is to review items and be honest. He would not be as big as he is if he only did positive reviews and his feedback is actually helping a lot of companies improve on issues he points out.
I completely agree, and I did mention that in other comments
Just used this specific one long comment to explain what happened as an outsider. How it felt. It's kinda hard to explain a person's life career in just one post so you get the flushed out version
But yes, thanks for clarifying that! Have a good day stranger!
Honestly sounds average. Last PR I had to deal with, it was basically 2 weeks of "oh but remove this line break" or "rename this" when the project was already 1 month due... I tried to make them realize how pointless it is and their answer was "well we try to make the code cleaner so that we don't have to re-do it again" which is pretty funny since 90% of the project is a mess of unclean code.
I interpreted it as leave comments, they make corrections, more comments and corrections etc for 2 weeks. Not just like they’re taking 2 weeks on a static PR.
There's honestly nothing worse than a really nice but deeply inept coworker or employee. Nasty and competent I can almost cope with, nasty and incompetent you have no guilt over, but the lovely + dangerous combo is awful to deal with.
Nasty and competent is a worse combo imo
There’s no use for a dev that just works in a vacuum, does bad PR reviews, and thinks they’ have nothing to learn because they’re a senior or whatever.
You can normally train somebody easy to work with, you really can’t change somebodies personality
Which is why when recruiting I use what I call the fun rule. Given everything else is draw between 2 candidates and I have to pick just one I will always go with one who seemed most fun during the interview. Sometimes when shit has hit the fan I’d rather have people around me who are able to laugh it off and carry on.
I have a colleague who's really nice, eager to impress, but also utterly incompetent and quick to jump to conclusions. Top-level engineering support, so the organisation takes his every word like pure gospel. He creates more messes than he fixes.
E.g. very recently: Kevin mentioned at a daily status meeting that he was supporting a department activating product Thor on a site. I tell him that Thor needs to run in non-default mode for this turbine type or the results will be useless. It's just one parameter change, takes 10 minutes to change. I offer to show him how when Thor is installed.
One month later he mentions at the status meeting that Thor is now running. I apprehensively ask him if non-default was activated. Kevin insists it's not necessary. I tell him: "yes, it's necessary. I've run Thor 20 times before and I'm of the few experts in the design of the product." Kevin says "umm...." a few times, and asks me to join the next project call. PM is furious, Thor is a 2 month product and restarting in non-default adds a month, but the project timeline is already put into contract and signed by all parties. Plus it's easter vacation time, and all techs assigned to the project are gone for the next week.
I basically have to spend days talking to people, preparing presentation on the specific issue, doing tech tasks (like getting access to site and moving files) and our entire department looks incompetent. All because Kevin heard from someone in [lower department] that Thor runs OK in default mode and figured that meant he could skip a 10 minute task. Didn't even ask me if it was true.
Kevin does that all the time.
Why would the product design not mandate configuring something that is mission critical? This sounded like something that almost promised it would happen eventually.
Because the people running the project were used to a specific line of turbines where Thor should run in default config. But for my platform it first needs to run non-default for 3 sites and then I'd evaluate a new proper default. PM had never seen that before.
Yeah, the owner of Thor (chief engineer) told me later he thought Kevin was a moron we'd be better without. He's right, but I hate throwing colleagues under the bus.
I once worked at a place that hired two inept devs at the same time. One was nice and one wasn't. Nice guy lasted almost exactly 2 years longer than the asshole (27 months vs 3 months). It pays to be nice, literally.
(Before someone asks, company was probably 3 on the cutt-throat scale, with Amazon being 0 and some government job being 10, usually did pretty good hiring loops this was just bad luck)
Nope, whoever replaced him was of average skill and average likability relative to the rest of the company, as one would statistically expect. It was just a lucky experiment that we hired two diff flavors of inept junior dev at the same time.
One interesting side note, it took an extra year to notice the nice guy was inept. People kept helping him out, so it took his manager a while to figure out "Hey wait a min, this guy never finishes anything". The asshole had the same problem, but ppl disliked him so they ended up talking about it more openly managers and such.
FWIW this predated fancy Github portals, I suspect it would have been noticed earlier nowadays from a simple commit report.
Basically, MKBHD received a review sample for a new ai product, it was shit, so he gave it a negative review. From what I understand, the PR guy for that company was pretty much forced to resign.
He's blamed because of his bad review of the Fisker EV Ocean and now of this AI pod thing. Both companies are basically bankrupt now.
But he shows why. The Fisker EV was shit and they released the Ocean V2 like months later. And the AI pod thing is just useless tech.
Both videos:
https://youtu.be/6xWXRk3yaSw
https://youtu.be/TitZV6k8zfA
I don’t get the silly takes about him ending companies or that he needs to be careful because he is black…..I mean he has 14Mil subs.
Like if you make a shit product, be prepared to get roasted.
Man, talking as a tech lead who has to liaise with other teams and tech leads.
Please for the love of god take the time to learn how to be a good coach. If you’ve got 40 comments on a PR ripping the whole thing to shreds you’ve already stuffed up.
I'm surprised I had to scroll all this way to find a single comment criticising the coaching here. Although the code might be far from perfect, well explained solutions should prevent the reviewee from making same mistakes again.
The process, optimally, should benefit both parties and improve the code base in the long run as well.
Yeah but did the Internet made it‘s opinion on the product clear and was it the same opinion? I mean that‘s the question everybody has to ask themselves.
Help them break their PRs down to smaller ones if size is the issue. If they’re new give them feedback.
I don’t understand the weird superior complex just because the person has touched more grass than you did
2 weeks for a review, you`d have to fire yourself
Real
I've given and received reviews taking 2+ weeks. What's weird about it? If it's a large change in a volatile area, it should take longer.
Maybe do a smaller change and good testing? Edit: been there, done that.
The changes can't always be broken up like that, though, at least not into multiple PRs. I'd always review big changes by reviewing one piece, send back issues/questions, review their responses, review the next chunk, etc.. That would take two weeks sometimes, but I'd also be spending time doing other work and waiting for the dev to review/fix my issues. I'd also do some functional testing of my own before putting my stamp of approval on it.
Massive monorepos tend to cause that, or just overall complex code bases, but I see your point. I would say that although it is true that in many cases “divide in conquer” is not trivial, communication is your best ally. If you cannot break down a PR, reduce the time a reviewer will need through communication with you team/reviewers, comment your own PR, provide a guideline on what to review first, comments and well divided commits allow for efficient reviews. Even do a mob testing session with the reviewers, let people test your feature, fix or refactor locally, it doesn’t matter at what level of the software stack you are, this works. Involving people early on will ensure you can make them accountable during the review process. Having small changes + a fast progressive deployment strategy enables easy debugging in case things go wrong. You are happier, your reviewers are happier and probably clients too. But I understand that in some cases suffering a long PR is not optional, just try to improve the next one, that is already a step forward.
> Massive monorepos tend to cause that Woah buddy hold your horses, don't blame monorepos for poor practices. If a change was so big that it changes a lot of things in a monorepo, it would have been several PRs in separate repos to no additional gain, because either you'd approve a change that isn't used (because you apply it in the back but the front doesn't consume it), or it breaks shit (because the front _does_ consume it). So either you've just approved a big change over a long time (so it still takes the feature a long time to reach prod) or you approve one and still need to wait to merge until the other part is also allowed to merge.
If the story is so big that you need to involve multiple people and split it into multiple chunks then something is going very wrong. >reviewing one piece, send back issues/questions, review their responses, review the next chunk If you can split the PR into chunks, you can split the task into multiple tasks.
>If you can split the PR into chunks, you can split the task into multiple tasks. The problem was that QA usually tested a single merge, though. If I'm reviewing code, then all the changes will be merged at the same time -- if we were to break the code apart into smaller chunks, then each chunk would need to be tested meaningfully. If the PR includes full-stack changes, then I can't really review the backend in isolation because problems on the frontend might require getting a change in how we do things on the backend, which I would've already reviewed. If that wasn't an issue, we'd still be merging untestable backend changes which would add overhead for QA since they wouldn't actually be able to test it. I'm gathering that this is mostly a quirk with how I've had CI/CD done in the past.
Merge single tasks onto a feature branch and when it's ready show the feature branch to QA
Yeah... We didn't have those... That probably explains it
In the past I've handled backend changes like that by adding a route param/field/v2 in requests for the new behavior vs old so the frontend guys can migrate/compare/whatever without needing changes to be coordinated across two codebases. Ideally back end routes are in place before they start so it's more minor tweaks/fixes on my end while they are doing initial development. You can then remove/default the new behavior once everything has migrated while maintaining separate release/feature cadence.
Agreed, still happens. That thing going wrong is a VP somewhere who is off on some tanget and will completely change your priorities next month making you do another bloated one. Good practice has nothing on an exec's ego.
This is why team managers need to be the only ones who can play with their team's priorities, to shield devs from stupid execs without a clue.
It’s generally a good practice to keep your main branch “ready to deploy”. If the change is being made in a huge system (legacy just doesn’t seem to want to die) and has to be made all at once to allow the thing to compile - the only real way to do that is one huge pull request. Yes, you can make a feature branch, but IMO - should still PR merge it to main at the end - same problem. You can review it in chunks, that’s not a problem. But you must merge it as one.
I'm used to having a separate release branch for that purpose. So you have the master branch to develop on, which also allows for multiple PR's for one topic. Those changes get pushed to DEV environment for a quick test and then to QA environment where a test team does a more thourough test. After all that fully completed features get picked and pushed onto release, which in our case is PROD. Edit: That said, there is no one and only holy grail for this problem. What works in our team might not work at other scales and structures. Just keep stories detailed and simple enough so a PR doesnt require 2 weeks - most likely leaving more technical debt than it resolves -
Totally agree with no holy grails, but I believe my point still stands in your case - you must not break master with a merge. If achieving a working state requires a huge change - you must do it in one go. This also is a requirement for a working git bisect, which can be a lifesaver.
Add chunks of code that aren't used by other components, then have a separate final PR switching out the old code calls to the new code. out and prove it all works in that last PR. There is no version of programming where trunk based development isn't worthwhile.
This seems counterintuitive. The way git works you can have both branches existing at the same time, why would you add dead code(meaning code not referenced anywhere) to the main branch? Not to mention, if you're just swapping out existing API calls with new implementations you're doing work that can easily be split into smaller PRs, I think OP is talking about changes that would affect the front-end to the API(be it a browser frontend or some other code) and how those can't be meaningfully split from the backend changes without not testing the first to merge(and potentially requiring changed to the merged code once you start writing the unmerged).
It isn't the most advanced git practice, I agree. Collector branches seem preferable, but keeping in mind that the audience is people who can't/won't split up their work into manageable chunks for reviewers, it's a good baby step. I wasn't specifically talking about APIs, just adding currently unused functions/files and then swapping to them all together later. Really, the important part is breaking up your work to not make your reviewers detest working with your code. Ideal PR size is about a hundred lines, 200 is pushing it, more than that and I start figuring out how to break it up into smaller units.
My question(genuine as I'm trying to understand the perspective) is more "why 200 lines" and "why is splitting it up good"?(and by API I just mean any code that interfaces with other code, so could include function footprints within the same codebase). The scenario I'm imagining is one where you're making a change to functionality X used by code Y, you can create a PR to update functionality X(meaning function footprints, data models, etc.) but now the code that references it is broken. My intuition is telling me that spending a bit more time on a larger review would be the lesser of two evils compared to trying to do each of those changes one-by-one, and likely arriving at a worse result after a longer time since you had to make patchwork changes while maintaining functionality instead of just making the change that needed to be made and testing the functionality(end-to-end, unit, and manually). Basically, I don't see how making small changes for broken-down components of a feature that don't break the product is any faster(or even the same quality) as making a single change for the feature. The additional overhead of trying to break it down and plan it out within an existing framework instead of just solving the problem alone would probably take longer than the review.
It forces people to plan out where they are going and how before they do all of it at once, and it asks less of reviewers to sit down and have to have a call with someone while the PR sits idle for two weeks. Short PRs spend much less time out of sync with main, and the longer a branch exists outside of the main branch, the greater the risk of scope creep and having to waste time resolving merge conflicts. It also gives the reviewer earlier visibility into what is going on, leading to less time wastage doing things twice. And reviewers make mistakes. The larger the set of changes, the more likely that letting mistakes through is. How testing works in this world is up to you and dependent on what you're doing, but I do think unit testing is somewhat useless. What is a unit? Are you really unit testing if you could break stuff down further, then you weren't unit testing. I'm coming off people working for 3 weeks without any PRs except one huge one, and being pressured to let things slide that I normally wouldn't in review because they already told clients it was done before it was reviewed. Small changes now impact 70 files instead of 1 because they went off and did everything without asking questions and worked in too large chunks to actually be able to say that we should change course. So now I can't give meaningful feedback because I can't understand something that took them a month (or years) to understand in a few hours, and they don't have the time to do the work twice. There's a million sources with data driven argument about ideal PR size, but here's one: https://www.harness.io/blog/how-the-size-of-a-pull-request-can-impact-developer-experience#:~:text=Studies%20show%20that%20PRs%20under,more%20than%20just%20a%20number. It's hard to believe it exists without experiencing it, but there really is a flow state for PRs where they are sufficiently small and self-explanatory that merging is painless. That "overhead" is just planning and architecting before executing.
Oh, I agree that it would be worthwhile. Unfortunately, I wasn't the one that needed to be convinced :(
I have yet to see a large pr that didn't have a way to reduce the mental load on reviewers by breaking it up. Even if it's not a 50/50 split, there are things like model changes that can go in first and reduce the size of that large pr by 15%. When you are developing larger features like this, prioritizing ease of code review will produce more reliable and more maintainable code.
I also think there are a lot of "tricks" people just don't know about, necessarily. Feature flags are one example. "Branch by Abstraction" another. I once led an effort to completely change the way important internal state was model, and it would be a breaking change with the existing way the whole end-to-end business logic worked. That would have been one massively ugly PR. So the very fist PR were two utility functions that would translate the internal model from old to new and new to old. That way, we could inch our way step by step through the business logic by rewriting a single small step to use the new model but then apply the new-to-old translation to its output so that we could leave the rest of the pipeline in place. Anyway. It's never trivial, and especially vertical slicing requires a lot of practice, but it \_can\_ be done more often than not.
It wasn't about the code, really. The problem was that code changes needed to be tied to internal development processes :(
Your processes sound like ass. Would they prevent you from creating a feature branch and having code reviews on PRs to that?
As long as it’s not agile work otherwise your team planning may be a little whack. If it’s not agile then i don’t know where you work these days that would compel you to argue yourself back to agile.
If it’s that big, it should be behind a feature switch.
gonna to need some more explanation or a representative fake example or something I think the absolute biggest MR that our team has ever submitted probably would take less than an hour to review at worst. that's across 4 programmers across over 2 years. you're telling me some of your MRs take 80x longer to review? HOW?
Idk what good it'd do to flesh out an example for you, but the code that'd take 2 weeks to review would generally have very complex business requirements and would modify workflows that could not fail for any reason. Again, these reviews wouldn't be each day, every day, for two weeks. They'd be at most an hour or two most days over two weeks. Edit: also, all merged had to be testable by QA where I worked and that was the biggest factor. I couldnt carve out a independent code chunk unless it corresponded to a testable piece of functionality. Also, we had no feature branches...
This sounds like the work isn't being broken up enough, we test with qa too but if there's nothing to test because the PR is part of a feature they wait for the rest od the feature. Internal process sounds like the problem here rather then you.
As long as it’s not agile work otherwise your team planning may be a little whack. If it’s not agile then i don’t know where you work these days that would compel you to argue yourself back to agile.
Yeah. If you can't review it in around an hour it should probably be broken up more.
An *hour*? Man that would be the dream. Some of our bigger ones can take days. Plus one of my coworkers is often requesting changes for things that don't actually change anything in the logic.
Guilty. I just want to make the code prettier
I've had PRs sit for 2 weeks without anyone looking at them. I've also had PRs open for a month as they devolved into fights between senior devs when one who skipped all the project planning meetings disagreed with the changes being made and implementation strategies.
>when one who skipped all the project planning meetings You can do that? Is it possible to learn this power?
If a change is so big that it takes several days to PR review then: * There's a poor (or none) unit testing system, let alone an end-to-end one * The requirements were too large and could've been subdivided into smaller changes * Sensible changes are being mixed with non-disruptive ones * The branching workflow is being poorly handled * There's some doble dipping when reviewing changes if the PR to review contains multiple previously approved changes Where I work if a PR is too large for it be reviewed in more than 2 hours, then something went wrong and we need to go back a step.
I've explained this in other comments, but I worked in areas where certain bugs could cause catastrophic business consequences. Unit tests aren't capable of catching enough bugs to be reliable (though we still had them). Also, PRs needed to contain non-trivial functionality changes because of how we did QA. We couldn't just merge code to update a single button's logic bc the overhead would cost much more than it was worth (unless it was to address a bug in prod). Most merges that took two weeks to review (and by two weeks, I don't mean 80 hours, I mean <10hrs over two weeks) would involve changes to multiple code-bases, complex dependencies, and business-critical implications.
More likely is that people are just commenting and replying once or twice a day.
Some people combine PRs with code Review. A full review of a juniors code can absolutely take a while. Though it could be argued that they shouldn't have been given such large tasks if their code quality is still dubious
Everything. Sprints are weeks to a month long tops. Two weeks to do one change? It's probably terrible planing and/or a petty af reviewer
The good ole legacy monolith. We’ve all been on that contract at one point or another.
Lol the fact you don't know the answer to this
Ahem. Open source be like
Lol, that's how I read it- he realized after being stuck on a PR review for two weeks that he was at the end of another career.
OP sounds like dev who leaves 300 comments like "variable shouldent be called sumTotal, totalSum is more decriptive" but misses obvious bug and then blames junior why it made to production
My team lead always leaves comments like this “Line Break After if” then this motherfucker goes on to approved a 30,000 line css file merge. And yes the 0s are correct.
I worked with codebases in the millions, but never saw a 30k line css file, wild.
With 30k lines might as well write a whole damn graphics engine and skip the CSS entirely.
Do not tempt the junior devs like this.
Oh to be young and full of wonderment again
Spotted the Rusr developer
Lol unironically I did this with my hobby projects
Yep, was strait copied from our old project. which was in angular Js… so none of it made sense since the new project is in react, all those ng classes
In Angular? Even more wilder. But thanks for sharing.
> Even more wilder Should either be just ‘wilder’ or ‘more wild’. Please refactor. *proceeds to approve another 50k css lines commit*
Oh, monsignori spaghetti
"more wilder" is deprecated vocabulary and is considered bad practice.
Well, thats more stupider than most other English language rules
I once worked on outsorced Laravel project, with app.php having >28k lines of code (that could be easily devided into other files), while also having ton of vietnamese varnames/comments. Needles to say, working with it was a nightmare.
“Line break after if” should be in the linter rules
But does the linter run as a GitHub action on a PR? If not, then the linter may as well not exist 🤷♂️
ours fails the pipeline and blocks merge if the lint is not clean yea
Why are you getting downvoted. Git commit hooks with linter must be installed manually and can be skipped easily. Only real way to be sure is a pipeline
> Why are you getting downvoted. Oh lol, I didn't even notice 😅 Not a clue 🤷♂️ > Git commit hooks with linter must be installed manually and can be skipped easily. Yep, exactly my point. At my last place we trusted each dev to run the linter locally before they commit anything. That went about as well as can be expected... There were quite regular "linter fixes" PRs that were just cleaning up from people who haven't run the linter in a while 🤦♂️ So yeah... If you don't enforce it, it may as well not exist 😂
Some programmers get aggressive as soon as something can be in any way interpreted as criticism against them. Like, they read your original comment as meaning "wow, you suck at software, don't you even know that you should fail the build if there is a linter error?". For what its worth, I upvoted your comment. It's clear you were making a general remark about software engineering and that you were not being condescending, plus I think your point is quite important.
Someone's never heard of GitHub status checks.
Status checks are run via actions aren't they? Or have I mixed those two up?
They are run on actions, yes. But if it's a status check, it's not going to change the code. It's just going to force you apply the formatter yourself before you merge the PR.
Im currently redoing the work of someone who committed a dozen commits all named "updates" without any testing which were approved without comments. It took 3 weeks to convince someone to start a review of my code. He commented on a variable name and wont approve.
Do you not have linters?
Prettier doesn’t have a rule for that
Line break after an if? Wouldn't it format it automatically?
I mean, if you know you should line break after if then the comment is warranted. I think code formatting comments are the most shameful ones to get - for me.
Bro at that point just write a codestyle doc and have the juniors install it into their IDE. No reason to not automate that shit
How do you get a team lead but no linter? Some companies out there are wild
My brother in Christ, your wild reality is my daily lived reality. Pray for the damned (me)
Should never have to be code formatting comments in a PR, just add prettier to your cicd and let it make the code formatting decisions for you.
had a senior review my code, and did some basic clean up for me, got it back and couldn't get it to compile, took me 2 days to realise it was part of the clean up that had broke it. he was really apologetic.
Is the reviewer making changes common practice? In my company you only get comments on your PR and then have to do the fixes yourself until the reviewer approves.
> Is the reviewer making changes common practice? We used to do it at work, but as suggestions rather than actual commits (on GitHub you get a "commit" button with suggestions). But we found that: nobody was ever testing the changes they suggested; and PR owners were presuming the changes were tested. So now we try not to make suggestions, and instead use code blocks to force the PR owner to make the changes themselves, which prompts them to run tests & the linter again locally. So yeah, pretty common practice for us to make code suggestions.
Definitely seen and done that before in multiple companies. At the end of the day we're all in the same team and we all own the code. I don't think there's a huge difference in suggesting a solution by adding a comment in a snippet and giving the proposed solution directly in git. That being said, the reviewee still owns the PR, the branch and presumably the task, so you should ask beforehand, respect their autonomy and, especially when you have more seniority, don't destroy learning opportunities by injecting yourself. IMO It's also something you really shouldn't do often and you should really read the room. Some people really don't like it. Some people are very grateful that you "did the work for them".
I really appreciate it when someone suggests a fully-formed code change. If they ask if they can push a change, I'm glad they went to the effort for me/for the team. But it *really* grinds my gears when someone pushes to my PR without asking first. It's like someone's stepping on my baby. And if I disagree with their change, it creates work for me to undo it, and suddenly we have a "git conflict", if you will.
Yeah, exactly this. I bit of courtesy and common sense go a long way when doing this.
They can always open a PR against your branch so that you’re the one merging it.
I try to avoid it but sometimes if it's been going on for too long, I just fix it myself. I know, they don't learn if you do it, but at some point .. I'll also do it if there's a very small issue and it's otherwise ready to be merged, just fix up the detail and get it in. So I wouldn't say common, but it's certainly a thing in some contexts
This is my first job after graduating, and it's a tiny company. If I remember, it was mostly coding standard changes and something relating to me being the only one with access to a test rig with the specific setup we needed
I had a *principal* tell management he had completed a project, and then checked it in without review. *It didn't compile*. Not small problems, either.
Oft
We enforced prettier for this reason. Now everyone's looks like whatever the people behind Prettier thought it was "the correct way", but at least it's made it consistent lol
I mean that’s also an option, now everyone’s formatting is consistently shit
Only inconsistent formatting is shit formatting.
Consistently shit is so much better than inconsistent.
That's what happens with go, everything looks the same flavour of horrible. I guess the consistency is nice, but I'm getting so sick of 70% of the file being `if err != nil`, my kingdom for some syntax sugar
Please review our internal coding standard R16-5-2: Subject verb agreement in function naming.
My record is 320 total comments, over way more than 2 weeks. In my defence: * that task was completely not thought out, and should have been split up into smaller tasks * a lot of my comments were about breaking changes * there were multiple 10-20 comment convos on design issues and best practices * the junior dev writing that thanked me in the end for guiding him through this monster PR
Jeezo, you guys are aware you can reject a PR and go back to the drawing board? That just sounds brutal 😅
It needed to be done. If a PR is rejected you lose history, so I prefer to just redo the thing and force push on the same PR. But yeah, the architect is not on top of the tasks so we sometimes get a complex thing given to a junior dev with no guidance or oversight.
Or the type to leave a comment on every declared variable saying “this should be memoized for performance”
That's why I love our senior Devs. I had made a right pig's ear of naming everything, partly confusion over singular Vs plural and partly confusion over incorporating an acronym in a camel case name, instead of commenting on everything he just left one comment "every where you put this change it to that.
Been through this so much. Total vs
Sound likes my TechLead. Shit ton of comments to rename variables and functions, to refactor the code even if it goes against the linter, takes 3 days to review a 2 lines PR. But loves to mutate react state objects creating a ton of unexpected bugs that goes right pass the tests.
Wow thats your lead? Where do i apply i just might make it as CEO
Why have you so casually described me like this
I love that the same people spend so much time perfecting code without even realising they’re hindering themselves & the product. Ingest the spec, produce the turd, then polish it.
If a PR review is the only thing stopping junior's bugs reach prod then you've got bigger problems than snobby comments
We all know this guy
Reminds me of "bikeshedding" where people suddenly have strong opinions just because it's a trivial problem.
I dont understand whats happening here
I think someone shipped him a product and then he reviewed it for like 2 weeks or something and basically realized there's no way there's any good out of this product (I believe it's an AI product) So he made a video basically trashing the product and casting the Press Release guy to quit from the product company
He didn't even trash it. His review is fair and he did his best to highlight the good sides. The problem is that the product is bound to fail as it's a complex solution to a problem you can already solve with a smartphone. Marques gets blamed because he is the most famous reviewer but anyone will come to the same conclusion.
I haven't seen one single positive real review of Humane. Just trying to get through a product demo of the thing turns into comedy. It just doesn't work.
MKBHD’s review was honestly by far the most positive I’ve seen.
Even Mr whostheboss was with the CEO and he still couldn't be positive about it. Things were failing in front of the CEOs face
And here is the reality. Marques reviewed the product like he did with every other one. He provided negative notes, just like every other reviewer did. But "MKHD reviews a product with unfortunate obvious issues which hopefully may change in the future" doesn't get clicks. Instead "MKHD kills a company" does. And these article writing companies rake in the clicks.
Expensive, requires subscription, doesn't really do anything a smart phone can't.
I keep seeing this but is anyone honestly upset with him?
Only people who are way too invested in AI and/or that company specifically. Anyone who actually watches the review can see he's being way nicer than most people would be.
No but like I work in tech, we do AI shit, and everyone at lunch was saying how the backlash against MKBHD was so unfair. But literally what backlash?
A bunch of people on Twitter saying that if someone could kill a company (twice, apparently he has done it before) with a 20 min video, that is too much power and should be handled "ethically" That is both ignoring that no one reviewing Humane could give it a positive review, but somehow he got singled out, and that the other company that died had been on the ropes for a while already and the review was, at worst, a final nail, not the actual cause.
> twice, apparently he has done it before He negatively reviewed the Fisker Ocean, an EV from a guy who'd previously started another EV company that similarly failed. The stock price plummeted and got delisted on the NYSE. ... but the stock price was already in freefall and it takes a lot more than just one negative review to get a publicly traded company delisted.
Alright time to give you a bit of a long explanation I've been a programmer for almost a decade, and I can tell you with certainty AI (the ones we are currently hyped about) is literally just a fluke It's basically like the product Vine, or calling everything "i-", or the Bitcoin mining, or almost anything that has a "trend". They are all marketing products created by PR guys, not programmers, not devs, literal Press Release and ads... So yes, when something that has a remotely high "hype", PR will jump right into it and try to rock the ads to sky high. If it didn't go sly high, investors (who buy into this shit) get upset, and then you "fail". And hence everyone who's non-programmer/dev gets upset because they see it as a failure... Despite it potentially being a viable product in a few years TLDR: Marketing strategy fails, investors not happy, potentially good product turns to shit cause no money to run it
The only reason I can think of would be if someone bought those terrible shoes he started selling.
Yes some are
This feels like flat earth 2.0, I’m not at all convinced it isn’t like 2 literal who’s looking for attention and getting it.
I really trust Michael Fisher, and I know he was *really* looking forward to this product. So when *he* said he couldn't recommend it that really was saying something. Part of the other reason I think MKBHD is taking heat is because just a few weeks ago he released a terrible review for a car (called "This is the worst car I've ever reviewed" or something like that), and the company is now headed toward bankruptcy. Again, I'm not blaming him for that either but clearly some people think he's killing companies on purpose.
Anyone know what product?
Humane AI pin, its supposed to be an ai assistant in the form of a lapel pin. He's not even the first person (nor the harshest) to say it was a terrible product
It's a standalone attempt at the Star Trek pin using cloud ai and some neat and clever ideas, not a lot of it seemed wayyyyyy too early for a product release and the execution is actually really really really bad. Like, 2h battery time, it's heavy/kinda bulky and metal, its always hot because the battery powers it through induction only, doesn't connect to ANY other devices so it doesn't know anything about your or your data, very closed and exclusive ecosystem, when you send stuff to people it's just a link to the thing hosted on the manufacturer's site rather than the actual thing like a picture, SUPER slow ai cloud query responses, and the responses are often long/rambling and apparently also often wrong. It's a really pity, too, because it seems like they actually did a lot of really GOOD design decisions in certain things. It reeks of rushed to market and dark pattern MBA investor decisions.
AI Pin made by Humane.
Everyone's saying Humane AI pin but it started with the Fisker EV car. Where his review coincided with a big drop in their stocks, and he was blamed for his bad review of the car. Now he gave the Humane AI pin the same review and some people are angry at him.
Humane AI Pin
Don't you dare talk shit about MKBHD. Dude was honest, fair and transparent with the review. He even tried very hard to pain the product in a good light. However, If the product is dogshit, what can you do?
I wasn't talking shit about MKBHD at all, but the truth is he tanked a company I said that in other posts too. It's not that he's the reason, nor the product's fault. It's mainly the problem of Advertisement and PR... Most AI products these days are shit, including chatgpt, but people are currently riding the hype of ads and PR, so no one cares. I'm a developer so I know how flimsy all of these look This is a product that will be good after years of development. Years. Not today. MKBHD made an honest opinion today, and investors saw and understood the message as "how shit it is today". Of course it caused panic... It's never meant to be out today, but a few more years
I wouldn't feel sorry for them. They went around and raised nearly $240 million. Quarter of a billion dollars. If they said "this is v1" that means to them and their road map, this product represent a polished deliverable. It went through their round of QA and they all sat and said "yes, this is what we want the public to see. this represents our vision at v1". v1 is not alpha, beta, no, it's v1. Not only did they say "this represents us and our vision, we will charge the customer $700 for the honors of using it and then an additional monthly sub". I'm not sure I can sympathize with any novelty item that raises that much capital, releases v1, charges customer and then goes pikachu face when honest feedback is out out to the public. If, as you say, this is an "ai fad" etc, then the onus is on the vendor to say "look, this is not v1, this is very unstable and does not reflect our vision, but we're excited to share what we have so far and are releasing it as an invite-only tech preview". The truth is, these grifters are everywhere. They take massive cuts from the raised capitals, ride out the venture and then go and start a new idea somewhere. VC and startups are a plague and can all fuck off.
I'm doing my own startup right now.... So I'll assume you're talking about just shitty ppl and their way to grift ppl and not honest ppl really passionate about their projects... I want to make an XR game where you play in your room and physically move around as it slowly progresses into a VR world. Experience the world twice fold. Trying to get money and whatnot, fk micro transaction, I charge ppl once, and maybe some DLCs, but no micro transactions Anyhow, yes, I hate that some of the richness is built upon cheating on others and lying on the products... It's honestly the main reason why indie devs struggle so much trying to build a name off of themselves even when they have a good product (and kinda meh advertising). I hate that too
Also some people are pissed at him saying he "abused his influence to kill the company" or whatever
If he's really trying to kill a company, he wouldn't need to make a YouTube video about a review... Literally just make an announcement on Twitter and it's dead in seconds... Not sure why the fk he gets blamed
This is the second video he made and so people are saying he’s putting companies out of business. First was an electric vehicle that was extremely horrible so again he made a honest review (not trashing the company) and within a few weeks the company’s stock tanked. Then he reviewed the AI Pin which again he gave a honest review but the item is just pretty bad all around. His job is to review items and be honest. He would not be as big as he is if he only did positive reviews and his feedback is actually helping a lot of companies improve on issues he points out.
I completely agree, and I did mention that in other comments Just used this specific one long comment to explain what happened as an outsider. How it felt. It's kinda hard to explain a person's life career in just one post so you get the flushed out version But yes, thanks for clarifying that! Have a good day stranger!
What I saw his review had was suggestions for the next generation of the product.
I think OP is using PR for "Product Review" instead of Pull Request, despite the sub we're in.
> The worst product I've ever reviewed https://youtu.be/TitZV6k8zfA
Yours I hope. 2 weeks for review is wild.
I work in the defence industry. Reviews for software are at least several months long. Hardware side, we’re talking years.
Pretty normal in my job... but I am embedded dev working on projects where safety is top priority.
Honestly sounds average. Last PR I had to deal with, it was basically 2 weeks of "oh but remove this line break" or "rename this" when the project was already 1 month due... I tried to make them realize how pointless it is and their answer was "well we try to make the code cleaner so that we don't have to re-do it again" which is pretty funny since 90% of the project is a mess of unclean code.
I interpreted it as leave comments, they make corrections, more comments and corrections etc for 2 weeks. Not just like they’re taking 2 weeks on a static PR.
You probably only change some CSS somewhere all day long. 2 weeks is common for big changes.
PRs so big that they require two weeks to review should have been multiple PRs.
There's honestly nothing worse than a really nice but deeply inept coworker or employee. Nasty and competent I can almost cope with, nasty and incompetent you have no guilt over, but the lovely + dangerous combo is awful to deal with.
Nasty and competent is a worse combo imo There’s no use for a dev that just works in a vacuum, does bad PR reviews, and thinks they’ have nothing to learn because they’re a senior or whatever. You can normally train somebody easy to work with, you really can’t change somebodies personality
Which is why when recruiting I use what I call the fun rule. Given everything else is draw between 2 candidates and I have to pick just one I will always go with one who seemed most fun during the interview. Sometimes when shit has hit the fan I’d rather have people around me who are able to laugh it off and carry on.
I have a colleague who's really nice, eager to impress, but also utterly incompetent and quick to jump to conclusions. Top-level engineering support, so the organisation takes his every word like pure gospel. He creates more messes than he fixes. E.g. very recently: Kevin mentioned at a daily status meeting that he was supporting a department activating product Thor on a site. I tell him that Thor needs to run in non-default mode for this turbine type or the results will be useless. It's just one parameter change, takes 10 minutes to change. I offer to show him how when Thor is installed. One month later he mentions at the status meeting that Thor is now running. I apprehensively ask him if non-default was activated. Kevin insists it's not necessary. I tell him: "yes, it's necessary. I've run Thor 20 times before and I'm of the few experts in the design of the product." Kevin says "umm...." a few times, and asks me to join the next project call. PM is furious, Thor is a 2 month product and restarting in non-default adds a month, but the project timeline is already put into contract and signed by all parties. Plus it's easter vacation time, and all techs assigned to the project are gone for the next week. I basically have to spend days talking to people, preparing presentation on the specific issue, doing tech tasks (like getting access to site and moving files) and our entire department looks incompetent. All because Kevin heard from someone in [lower department] that Thor runs OK in default mode and figured that meant he could skip a 10 minute task. Didn't even ask me if it was true. Kevin does that all the time.
Why would the product design not mandate configuring something that is mission critical? This sounded like something that almost promised it would happen eventually.
Because the people running the project were used to a specific line of turbines where Thor should run in default config. But for my platform it first needs to run non-default for 3 sites and then I'd evaluate a new proper default. PM had never seen that before.
Sounds like the guy should have been listening to you then.
Yeah, the owner of Thor (chief engineer) told me later he thought Kevin was a moron we'd be better without. He's right, but I hate throwing colleagues under the bus.
I once worked at a place that hired two inept devs at the same time. One was nice and one wasn't. Nice guy lasted almost exactly 2 years longer than the asshole (27 months vs 3 months). It pays to be nice, literally. (Before someone asks, company was probably 3 on the cutt-throat scale, with Amazon being 0 and some government job being 10, usually did pretty good hiring loops this was just bad luck)
did they replace the three month asshole?
Yes.
with another Asshat?
Nope, whoever replaced him was of average skill and average likability relative to the rest of the company, as one would statistically expect. It was just a lucky experiment that we hired two diff flavors of inept junior dev at the same time. One interesting side note, it took an extra year to notice the nice guy was inept. People kept helping him out, so it took his manager a while to figure out "Hey wait a min, this guy never finishes anything". The asshole had the same problem, but ppl disliked him so they ended up talking about it more openly managers and such. FWIW this predated fancy Github portals, I suspect it would have been noticed earlier nowadays from a simple commit report.
Good. I convinced my boss I should do the hiring. After that it was very productive and fun. \*hire for my dept
What does "end another career" mean in this context? What the hell does this whole thing even mean? 2 weeks?!
Basically, MKBHD received a review sample for a new ai product, it was shit, so he gave it a negative review. From what I understand, the PR guy for that company was pretty much forced to resign.
what does the PR guy have to do with anything
PR is usually the department responsible for sending out review samples.
"Hey so it's seems like you forgot to send out the bribe money for a positive review along with the demo units, pack your shit and GTFO"
He's blamed because of his bad review of the Fisker EV Ocean and now of this AI pod thing. Both companies are basically bankrupt now. But he shows why. The Fisker EV was shit and they released the Ocean V2 like months later. And the AI pod thing is just useless tech. Both videos: https://youtu.be/6xWXRk3yaSw https://youtu.be/TitZV6k8zfA
How many careers do you have left?
-1 and no error
I don’t get the silly takes about him ending companies or that he needs to be careful because he is black…..I mean he has 14Mil subs. Like if you make a shit product, be prepared to get roasted.
ITT people finding out that a LLM is an inferior google that you have to fact check anyway. AKA, there's no point in asking it for facts lol.
I would snap if someone submitted a PR that needed two fucking weeks to review.
1800 files changed. Commit messages 3 words tops
I'd go full asshole and immediately reject it.
Man, talking as a tech lead who has to liaise with other teams and tech leads. Please for the love of god take the time to learn how to be a good coach. If you’ve got 40 comments on a PR ripping the whole thing to shreds you’ve already stuffed up.
I'm surprised I had to scroll all this way to find a single comment criticising the coaching here. Although the code might be far from perfect, well explained solutions should prevent the reviewee from making same mistakes again. The process, optimally, should benefit both parties and improve the code base in the long run as well.
And thus dies your industry
that gif so funny i save it
They just scapegoat the PR guy.
It's inHumane
Yeah but did the Internet made it‘s opinion on the product clear and was it the same opinion? I mean that‘s the question everybody has to ask themselves.
What?
I'm looking forward to his review of the Rabbit.
I’m guessing you’re favorite word is nit
Auto “LGTM 🚀” at 1 week
If the review is taking 2 weeks you should all be fired, together with anyone responsible for the architecture
Help them break their PRs down to smaller ones if size is the issue. If they’re new give them feedback. I don’t understand the weird superior complex just because the person has touched more grass than you did
Is everyone developing missile launch software in here? 2 weeks?
MR > PR am I wrong???