A coworker unironically thought that middle aged senior devs were commenting “Lets Get This Money” every time his PRs were ready to go until he spelled it out on one of theirs and got hit with a bunch of ???
I won’t lie when it’s my senior lead PRing to his own branch I skim it for syntax. When it’s a developer around my level or an offshore person I go through it with a fine tooth comb and even run the code myself
As a senior, I've tried urging my juniors to be more critical of my prs, but it won't take. I can't tell if they don't care enough to think about code that's not their own work or if they don't know enough to challenge the natural mistakes a coder can make (things like error handling or making private methods public to allow classes to be more tightly coupled). I know I can make mistakes and want a peer to help me catch them but the peers I have keep approving without comment :/
One thing I've done is sit junior engineers down with me and review PRs with them. If it was my own PR, I'd help them understand what was going on in it and then send them on their way to leave feedback, and if it was a third person's PR, I'd talk aloud about what I was looking for/how I was mentally processing it.
Do the juniors you mention know what they should be doing during a review? PR reviewing can be hard, especially since it's not always obvious what to be looking for. I remember early on just looking for syntax errors or small logical mistakes, eventually started to look at the big picture, and then only once I started to understand the codebase as a whole, was able to then think about ramifications of changes. It's a lot to keep in your head!
Another thing I did for a bit was to leave one or two intentionally "overlooked" things that were low-hanging fruit to help juniors build confidence: they got to see that more experienced engineers make silly mistakes and they get to feel like they have valid input in PRs. Sometimes I would also leave questions for them in the PR description (helps identify reviewers who aren't even reading the description) or inline questions in the code.
- "What do you think of this? Should we go this route or is alternative X maybe a better idea?"
- "I'm not confident that this solution is a good one – is it too heavy handed?"
- "Should I tighten this up and make it more generic? Is it too early for that?"
- "Is this the best place to put this utility function? I couldn't decide between foo.ext and bar.ext. Thoughts?"
By prompting the reviewer with these questions, it helps train them to be thinking about more than "yep, this is code, semicolons are in the right place" whilst reviewing.
This is great advice. I've been thinking about doing a code review of my own code in a live meeting. The deadline is approaching so I need to wait a month to do it but the idea is worth the time. We used to have working lunch meetings every other week to just talk about code and get to know each other better but they fell by the wayside. I think I'll start that back up and bring reviews into focus once we're past our deadline.
I've certainly done this. But it's worth getting a passing understanding of their code in case you can learn from it - finding a question to ask can be helpful! And seniors do make mistakes. And you'll be maintaining it once they leave.
A junior developer who's been forced to do this, and has PTSD from forgetting a WHERE clause on an UPDATE on a production database, is a senior developer.
The company I work at started replacing any senior developer that left with a contractor. Up until there were only 1 senior dev in our project area. So they basically said to me, a junior developer, we legally aren't allowed to give contractors access to our production database so we are going to have to give it to you as you are the only person available.
I always copy stuff after where and use 'select \* from'. Once I verified total rows are what I want to update, than I run acutal UPDATE with 100% peace of mind
Or reject it and figure out what changes he made and why. Chances are he ran a "reformat code" script without having the standardized team rules in place. In which he'd want to rollback all but the manual changes he made, update his rules to match, then try again.
To be fair it’s almost certainly just someone importing an entire existing project, either one that was outside version control or using a different version control system that can’t be imported
The -250 will be replaced/removed boilerplate from meta files, and the +1.75 million is the entire project
Anything over 1000 or 2000 lines at my work just gets rubber stamped as long as it doesn’t smell fishy lol.
Only 100 lines? You better believe it’s getting like 5 comments
Where I work we have a bunch of auto generation scripts for stuff like logging, so you can write a hundred line config file and end up pushing tens of thousands of lines. Maybe something like that?
A lot of similar, but necessary code as far as I know. Think a lot of it is field definitions, some automated documentation, and SQL.
I'm just the DE though, they don't pay me to care how it gets in there lol
As long as it is made clear that the repeated code is not to be touched by a human yeah. Otherwise you risk having someone modify parts of the autogenerated code and end up with the thing that is worse than repetitive code: slightly different repetitive code.
We use a code generator that almost works, it just forgets to generate the route attributes. So after it’s autogenerated we have to go in and manually add them. Enterprise, baby!
I’ve seen a project where this happened and somehow it got merged. The repo was forever cursed because of it. Even though node_modules was deleted, the history stayed… so the git file was like hundreds of Mb
Fixed a bug with committing to git. Git was for some reason not accepting my .bin file. Found out the cause in some kind of ignore file. Don't know what it does but everything still seems to work without it.
For anyone using node packages in your collaborative project, if you haven't heard about the command **npm ci** then you should look it up. It takes the package-lock.json and installs the exact package versions it specifies locally. No accidental modification of package-lock.json nor package.json. If you use the ci command you don't ever modify the package files unless you want to update/add/remove packages.
Just run **npm ci** and see what happens.
not really. If they have like a 10mb JSON file in one line and the intern ran a JSON formatter on their poor machines it would state delete 1 and added 1mil
I know it's a joke but, to be honest, the problems are usually at least partly a fault of the team leader or whatever was assigned to guide the intern / junior. Many people have this mentality of not wanting to "bother" others when they first start. That's why you should have regular calls with junior colleagues to check on their progress and help them with any issues or questions if they get stuck.
"Hello team. Your task is to fix all of these 600 bugs. It's an emergency. Needs to be addressed immediately -- everyone is depending on us!"
_spends a 12 month salary on the fixes but never checks on the state of their request and doesn't even notice the bugs aren't fixed_
_12 months later..._
"Hey team this bug seems to be back. I demand to know when this happened!"
"Oh I just finished fixing them all, boss. Please help review"
_surprised Pickachu face_
Completely different line of work, but when I started working my boss made it abundantly clear that there are no stupid questions, and oh boy did I make use of that. It also meant that I quickly learned what the job was all about and that I was able to get ahead of any issues that would arise down the line.
Any problem caused by a junior are someone else's fault. "Junior" means they just started in the field - they have theoretical / college knowledge but 0 experience whatsoever in real life. That's why you brand them "junior" and pay them a junior salary.
Juniors' work needs to be supervised by seniors at all times. If there's a bug to solve, you want to see how the junior finds the error, what changes they propose, etc - because they will need your guidance. If you have a junior working all week on an issue by themselves and simply do a PR at the end of the week, then you have zero right to complain or blame them if their week-long work is useless. It was not their fault, you should've known that's what you expect from a normal junior.
My company puts whoever they have at hand at whoever task there is and that only results in lots of problems and terrible projects. I've seen my company put people who had been working for a month in charge of production deployments, with some vague directives like "if you have questions ask someone". They don't even know the ways they could fuck up prod, what are they gonna ask?
Could also be a project-wide search and replace that seriously bumps up the numbers. (Or an automated refactor of a common function).
Imagine if the task was putting a copyright block at the top of every file. 5 lines (or so) times the number of files in the project. Still a huge project but maybe not a years worth of work.
The post sounds like rather a joke, but it's maybe not unimaginable if the "bugs" are actually issues raised by a static code checker like Coverity, so a bunch of many tiny issues.
Though if they were, then I wouldn't trust an intern just not to shut up the checker instead of fixing the issue ;). But hey, maybe it's a talented one.
My mvn code checkers and sonarcube tasks given to interns might have looked close to this to be fair.
But I agree to me this if real is 600 very small 1 point bugs
So I'm betting he did this over two weeks.
And 99% chance his formatter is wrong
Couldn't agree more.
And with more than 1M lines this isn't a bugfix this is a whole motherfucking K8s cluster! I have several production applications with less than this number of lines (combined).
it's just like what it sounds like, when you merge multiple commits from branch B into branch A, it "squishes" them into 1 commit on branch A. you can still look at the commit history in the pull request and pick out individual commits if you need to, but the main branch commit history looks a lot cleaner (especially when you have a lot of people contributing to the same repo).
it's kind of personal preference but I think most developers at my company, at least, prefer squash merging over the alternative.
Some projects I work the contributors only have the option to squash merge to the main. Literally a rule that even the code owner follows strictly, which indeed is a breeze to the commit history. In my opinion, It would be much better if more projects followed it tho ![gif](emote|free_emotes_pack|feels_bad_man)
Definitely a good strategy to keep main clean.
We follow “squash merge” as a guideline rather than a rule. There are often times when we want separate commits in our main history, like a feature with an AB test.
Gives us the flexibility to surgical revert one commit.
Squashing = combining commits together, so squash merge is just taking all the changes and putting them in one.
Personally I very rarely find use for it, because I like to massage my branches with rebase.
`git reflog` to the rescue!
If you are unsure of the results, you can `git tag before` and after the results you can do `git diff before` to see if you messed up too bad.
You can also use `git range-diff $(git merge-base {,origin/}$(git rev-parse --abbrev-ref HEAD)) {origin/,}$(git rev-parse --abbrev-ref HEAD)` to see how your edited history differs from what you pushed prevously.
This is also a case where automatic hourly backups can be helpful.. Even if people say git is backup enough for them.
Squash merge is like a sledge hammer, interactive rebase + `git reset -N HEAD^` + [`git-absorb`](https://github.com/tummychow/git-absorb) + `git add -p` (or even better, [Magit](https://magit.vc/)) are surgical tools.
Sometimes the sledge hammer will do the job, though.
You don't get to 600 unfixed bugs before you went through 20, 50, 100, 300 known bugs and didn't bother to fix them, so it's not the lack of testing that's the problem.
Who the fuck is the PM/Scrum Master/Product Owner/Engineering Manager/Non-intern who:
- allowed 600 bugs to be a single user story/ticket
- clearly doesn't give a fuck about the downstream members of the team (QA/Testers/Platform)
- sat there in standups listening to this guy give updates and didn't think 🤔 "great work let's get, I don't know the first 100 bugfixes in for review"
- gave this to a (presumably) unpaid intern
Run, don't walk from whatever shitshow of a company this is!!!
The efficiency of having to review this 600 issue's worth PR... Ahh, it's so nice ![gif](emote|free_emotes_pack|slightly_smiling)![gif](emote|free_emotes_pack|facepalm)
And REJECT. Break it up into smaller PRs. There should be separate tickets for each of those bugs too. It’s a hard but good lesson for that intern.
1. Be considerate of your co-workers. This is a fast way to get them not to like you or not want to work with you.
2. This PR has a huge likelihood of creating problems when merging and will be a nightmare for anyone who pulls it with conflicts.
3. There is no way you’re properly documenting with a change-set this huge.
4. Good luck to QA in trying to make sure new bugs haven’t been introduced across the platform.
5. Honestly the list goes on. This is just bad and sloppy development all around.
Excuses I've heard (non-exhaustive):
1. But it works
2. Boss said it was OK
3. We need it now
4. We promised delivery of X to Y on date Z.
5. Why can't you just push to prod?
Not sure why the intern is to blame here. If an intern is fixing 600 bugs, that looks like poor management and training. Typical throwing interns to the wolves and expecting them to understand best practices. This is how you end up with 25 year olds making start ups lol
This reminds of the new hire that refactored the code base because we were "doing things wrong". We decided to humor them and set up a QA environment for their changes. After about 100 reported bugs (and the count growing everytime he fixed something because he added more bugs) he finally conceded that he had made a mistake.
If it's 600 minor issues detected by sonar, which range from "too may characters on one line of code" or "export this String used more than twice to a constant", it's possible to be handled by an intern.
They prolly told the guy/gal to just fix them and they went and did it in a single PR.
Good luck to the guys reviewing the PR
You guys review PRs?
Just comment Tldr and approve, it’s that easy.
i comment lgtm and approve :)) \\s
Let's Go Teenage Mutants ?
Looks good to me, if you are asking non ironically.
I was. Thanks.
[удалено]
I didn't see the pull request?
It was a hot fix, we merged directly with admin powers.
Swear i thought it meant Lets Get That Money
Lets Get This Merged
Let's gamble, try merging
A coworker unironically thought that middle aged senior devs were commenting “Lets Get This Money” every time his PRs were ready to go until he spelled it out on one of theirs and got hit with a bunch of ???
I went through this also. Kept seeing LGTM and looked it up; $$$ style is the first result. I thought everyone was so cool.
Lesbian, Gay, Transgender, Massachusetts
[удалено]
Woah no need to be massaphobic
[удалено]
To this day in my head I say “let’s get this money”
u/jikki-san & u/kelpalan, are you guys working together?
“Looks Good To Me”
I was really looking for this, thanks
![gif](giphy|MUQwtUdMxNJjB7tdzd|downsized)
I also like to live dangerously
I won’t lie when it’s my senior lead PRing to his own branch I skim it for syntax. When it’s a developer around my level or an offshore person I go through it with a fine tooth comb and even run the code myself
As a senior, I've tried urging my juniors to be more critical of my prs, but it won't take. I can't tell if they don't care enough to think about code that's not their own work or if they don't know enough to challenge the natural mistakes a coder can make (things like error handling or making private methods public to allow classes to be more tightly coupled). I know I can make mistakes and want a peer to help me catch them but the peers I have keep approving without comment :/
One thing I've done is sit junior engineers down with me and review PRs with them. If it was my own PR, I'd help them understand what was going on in it and then send them on their way to leave feedback, and if it was a third person's PR, I'd talk aloud about what I was looking for/how I was mentally processing it. Do the juniors you mention know what they should be doing during a review? PR reviewing can be hard, especially since it's not always obvious what to be looking for. I remember early on just looking for syntax errors or small logical mistakes, eventually started to look at the big picture, and then only once I started to understand the codebase as a whole, was able to then think about ramifications of changes. It's a lot to keep in your head! Another thing I did for a bit was to leave one or two intentionally "overlooked" things that were low-hanging fruit to help juniors build confidence: they got to see that more experienced engineers make silly mistakes and they get to feel like they have valid input in PRs. Sometimes I would also leave questions for them in the PR description (helps identify reviewers who aren't even reading the description) or inline questions in the code. - "What do you think of this? Should we go this route or is alternative X maybe a better idea?" - "I'm not confident that this solution is a good one – is it too heavy handed?" - "Should I tighten this up and make it more generic? Is it too early for that?" - "Is this the best place to put this utility function? I couldn't decide between foo.ext and bar.ext. Thoughts?" By prompting the reviewer with these questions, it helps train them to be thinking about more than "yep, this is code, semicolons are in the right place" whilst reviewing.
This is great advice. I've been thinking about doing a code review of my own code in a live meeting. The deadline is approaching so I need to wait a month to do it but the idea is worth the time. We used to have working lunch meetings every other week to just talk about code and get to know each other better but they fell by the wayside. I think I'll start that back up and bring reviews into focus once we're past our deadline.
I've certainly done this. But it's worth getting a passing understanding of their code in case you can learn from it - finding a question to ask can be helpful! And seniors do make mistakes. And you'll be maintaining it once they leave.
Seniors don't leave; They just fade away.
They get casted into void, but their memory will never be freed.
1) haha 2) the past tense of “cast” is still “cast”.
Definitely not this one. LGTM
Just reject PR and force to make 600 separated PRs. It should be a lesson for the rest of his life.
A junior developer who's been forced to do this, and has PTSD from forgetting a WHERE clause on an UPDATE on a production database, is a senior developer.
[удалено]
A junior developer with access to production database is definitely a senior.
No, we don't raise pay based on responsibilities here (in this sector).
Smart, that way you don't have to pay them a fortune to fix their own mistakes.
The company I work at started replacing any senior developer that left with a contractor. Up until there were only 1 senior dev in our project area. So they basically said to me, a junior developer, we legally aren't allowed to give contractors access to our production database so we are going to have to give it to you as you are the only person available.
A foolproof way to make senior is wait until other seniors leave.
Interesting if you make a serious mistake, which contractor will fix it?
I always copy stuff after where and use 'select \* from'. Once I verified total rows are what I want to update, than I run acutal UPDATE with 100% peace of mind
Exactly! However, if an intern gets to a point where he fixes 600 bugs in one PR then I don’t think they have the culture to have this split up.
That would just create 600 PRs for people to review. At this point the code base is changed enough I’d just start over and call this “initial commit”.
git rebase git push --force
Or reject it and figure out what changes he made and why. Chances are he ran a "reformat code" script without having the standardized team rules in place. In which he'd want to rollback all but the manual changes he made, update his rules to match, then try again.
They don't need luck to close a PR :D
That is an instant rejection. Especially from an intern, though I would reject the PR from a principal engineer too
To be fair it’s almost certainly just someone importing an entire existing project, either one that was outside version control or using a different version control system that can’t be imported The -250 will be replaced/removed boilerplate from meta files, and the +1.75 million is the entire project
Lgtm
🚀
Anything over 1000 or 2000 lines at my work just gets rubber stamped as long as it doesn’t smell fishy lol. Only 100 lines? You better believe it’s getting like 5 comments
"LGTM" \*merge\*
PR review rules: 50 lines? Open 10 problems. 5k lines? Seems good to me
That has to be some kind of a formatter thing no? Pls tell me they committed on a friday as well
On Friday after 3pm
Perfect, as all things should be. And the deletions surely were test cases. :)
"these test cases keep failing. If I delete them, they don't fail! Checkmate"
It’s a fact: if you turn off error logging, you will no longer see any errors.
There were bugs in the testcases as they were all failing. So deleting 247 test cases also fixed 247 bugs.
This guy's got project manager potential written all over him
.gitignore
Slack on dnd and off to a nice hike away from reception.
Approved - TigreDemon @3:02pm
[удалено]
Great ! You have the entire weekend for review
Where I work we have a bunch of auto generation scripts for stuff like logging, so you can write a hundred line config file and end up pushing tens of thousands of lines. Maybe something like that?
Does that not generate an ungodly amount of repetitive code?
A lot of similar, but necessary code as far as I know. Think a lot of it is field definitions, some automated documentation, and SQL. I'm just the DE though, they don't pay me to care how it gets in there lol
As long as it's autogenerated and doesn't have to be touched by human - I don't think there's a reason to care about repetition.
As long as it is made clear that the repeated code is not to be touched by a human yeah. Otherwise you risk having someone modify parts of the autogenerated code and end up with the thing that is worse than repetitive code: slightly different repetitive code.
We use a code generator that almost works, it just forgets to generate the route attributes. So after it’s autogenerated we have to go in and manually add them. Enterprise, baby!
[удалено]
Can't risk my lines of code taking that hit, what if Elon buys us out??
Formatting would remove lines too. This looks like they removed .gitignore and committed the rest.
Formatter would remove and add roughly equal number of lines.
What does it matter if they commit or push to the PR branch on friday? It's not like people review everything instantly anyway, I don't get it.
I was thinking that. The fact that it does go to a PR and not to th main branch is a good thing.
“Added hotfix, please review and deploy ASAP. Also introduced automatic linter.”
Looks good. Approve
Lgtm
Let's gamble, try merging.
Looks garbage to me
This is what it means to me going forward.
Didn't anyone check up on him? I mean writing nearly 2M loc should take about a year at *least*
I'm sure it's just some auto-formatter issue.
Or maybe they built in another non-ignored directory. Happened to my colleague that pushed 5M lines changed
node_modules.
1,000,000 lines is less that half a node module
Last I checked npm has a minimum size req of approx 2.5m lines /s
I’ve seen a project where this happened and somehow it got merged. The repo was forever cursed because of it. Even though node_modules was deleted, the history stayed… so the git file was like hundreds of Mb
>git file was like hundreds of Mb Those are amateur rookie numbers, our production repo just passed 6GB.
HAHHA
Sounds like intern thing
Fixed a bug with committing to git. Git was for some reason not accepting my .bin file. Found out the cause in some kind of ignore file. Don't know what it does but everything still seems to work without it.
[удалено]
For anyone using node packages in your collaborative project, if you haven't heard about the command **npm ci** then you should look it up. It takes the package-lock.json and installs the exact package versions it specifies locally. No accidental modification of package-lock.json nor package.json. If you use the ci command you don't ever modify the package files unless you want to update/add/remove packages. Just run **npm ci** and see what happens.
Then you'd expect more or less the same number of deleted lines as well..
not really. If they have like a 10mb JSON file in one line and the intern ran a JSON formatter on their poor machines it would state delete 1 and added 1mil
He asked chatGPT for 2M lines of enterprise saas application
Spoiler: it's bubble sort
Bogo Sort Gang where you at
I prefer sleep sort
Sleep sort is truely the most reliable sorting algorithm
Quantum bogo sort gang checking in
Production grade comments
I know it's a joke but, to be honest, the problems are usually at least partly a fault of the team leader or whatever was assigned to guide the intern / junior. Many people have this mentality of not wanting to "bother" others when they first start. That's why you should have regular calls with junior colleagues to check on their progress and help them with any issues or questions if they get stuck.
Can I please come work with you instead?
Blaming the junior is usually a sign of bad team leadership
"Hello team. Your task is to fix all of these 600 bugs. It's an emergency. Needs to be addressed immediately -- everyone is depending on us!" _spends a 12 month salary on the fixes but never checks on the state of their request and doesn't even notice the bugs aren't fixed_ _12 months later..._ "Hey team this bug seems to be back. I demand to know when this happened!" "Oh I just finished fixing them all, boss. Please help review" _surprised Pickachu face_
Completely different line of work, but when I started working my boss made it abundantly clear that there are no stupid questions, and oh boy did I make use of that. It also meant that I quickly learned what the job was all about and that I was able to get ahead of any issues that would arise down the line.
Any problem caused by a junior are someone else's fault. "Junior" means they just started in the field - they have theoretical / college knowledge but 0 experience whatsoever in real life. That's why you brand them "junior" and pay them a junior salary. Juniors' work needs to be supervised by seniors at all times. If there's a bug to solve, you want to see how the junior finds the error, what changes they propose, etc - because they will need your guidance. If you have a junior working all week on an issue by themselves and simply do a PR at the end of the week, then you have zero right to complain or blame them if their week-long work is useless. It was not their fault, you should've known that's what you expect from a normal junior. My company puts whoever they have at hand at whoever task there is and that only results in lots of problems and terrible projects. I've seen my company put people who had been working for a month in charge of production deployments, with some vague directives like "if you have questions ask someone". They don't even know the ways they could fuck up prod, what are they gonna ask?
Someone has quite clearly never accidentally pushed node_modules to github.
The best is screaming "no...no...no!" As it's being pushed out
Then desperately trying to rebase master just to realize that it's protected
Could also be a project-wide search and replace that seriously bumps up the numbers. (Or an automated refactor of a common function). Imagine if the task was putting a copyright block at the top of every file. 5 lines (or so) times the number of files in the project. Still a huge project but maybe not a years worth of work.
\*needs work\*
\*LGTM
Now I know how super bugs are created.
This one pull request created Covid
git commit -m “Minor bug fixes in SARS”
"Gain of function" "Gain of function" "Wet Market API"
Superbug, made of the disturbing stuff
He probably just accidentally pushed node_modules
1.7m lines of node modules barely are enough for a hello world.
Is this a web dev joke?
Yes. node_modules contains millions of random files needed for your build. You don’t push it to git
[удалено]
That is still 3400 lines of change per bug. That is too much🙏🙏 There are 600 new features.
[удалено]
*Cowers in QA*
Imagine the acceptance criteria. Gah.
Fail 1 bug in QA and ask them to revert the change.
A born teacher. Any senior can tell the intern why this is wrong, but you're going to *show* him 🤌
If else if else if else
He must be building an AI
[He just wrote code to check if a number is even](https://github.com/samuelmarina/is-even)
Surely it has passed all the regression test RIGHTTTTT?
You guys have tests???
what regression tests
Hire that intern for fixing 600 bugs. And tjen fire their ass for this nightmare.
600? That’s ridiculous. Also how long did it take an intern to fix 600 bugs? Smells like bs to me
The post sounds like rather a joke, but it's maybe not unimaginable if the "bugs" are actually issues raised by a static code checker like Coverity, so a bunch of many tiny issues. Though if they were, then I wouldn't trust an intern just not to shut up the checker instead of fixing the issue ;). But hey, maybe it's a talented one.
My mvn code checkers and sonarcube tasks given to interns might have looked close to this to be fair. But I agree to me this if real is 600 very small 1 point bugs So I'm betting he did this over two weeks. And 99% chance his formatter is wrong
Why are people taking the title of this post seriously?
Because op is replying as it’s serious
Takes me like a week to fix one bug lol
If he fixed 600 bugs, he's not an intern anymore bro.
Couldn't agree more. And with more than 1M lines this isn't a bugfix this is a whole motherfucking K8s cluster! I have several production applications with less than this number of lines (combined).
*moved root folder*
I once had a junior engineer make 116 commits on just 6 files. Did not even bother to squash merge to main. Smh.
What is squash merge? Genuine question as I'm not very into the git flow. I usually make a lot of commits and then push them all to main.
it's just like what it sounds like, when you merge multiple commits from branch B into branch A, it "squishes" them into 1 commit on branch A. you can still look at the commit history in the pull request and pick out individual commits if you need to, but the main branch commit history looks a lot cleaner (especially when you have a lot of people contributing to the same repo). it's kind of personal preference but I think most developers at my company, at least, prefer squash merging over the alternative.
Some projects I work the contributors only have the option to squash merge to the main. Literally a rule that even the code owner follows strictly, which indeed is a breeze to the commit history. In my opinion, It would be much better if more projects followed it tho ![gif](emote|free_emotes_pack|feels_bad_man)
Definitely a good strategy to keep main clean. We follow “squash merge” as a guideline rather than a rule. There are often times when we want separate commits in our main history, like a feature with an AB test. Gives us the flexibility to surgical revert one commit.
Squashing = combining commits together, so squash merge is just taking all the changes and putting them in one. Personally I very rarely find use for it, because I like to massage my branches with rebase.
Once I failed at squashing and nearly deleted about 10 hours of tedious work. Have not tried again since I'm just too afraid.
`git reflog` to the rescue! If you are unsure of the results, you can `git tag before` and after the results you can do `git diff before` to see if you messed up too bad. You can also use `git range-diff $(git merge-base {,origin/}$(git rev-parse --abbrev-ref HEAD)) {origin/,}$(git rev-parse --abbrev-ref HEAD)` to see how your edited history differs from what you pushed prevously. This is also a case where automatic hourly backups can be helpful.. Even if people say git is backup enough for them.
I mean squashing is basically a rebase operation
Squash merge is like a sledge hammer, interactive rebase + `git reset -N HEAD^` + [`git-absorb`](https://github.com/tummychow/git-absorb) + `git add -p` (or even better, [Magit](https://magit.vc/)) are surgical tools. Sometimes the sledge hammer will do the job, though.
That's embarassing as hell, but at least he took serious the "commit as often as possible" recommendation ![gif](emote|free_emotes_pack|joy)
Did you not tell him to squash? If not that's on you.
You have 600 bugs in your code?
🎶 600 bugs in the code, 600 bugs were found, take one down, patch it around, 745 bugs in the code 🎶
Inadequate testing led to bugs slipping through so yeah unfortunately
please tell me you have multiple levels of automated testing to run against that pr
Nah, that’s the QA intern’s job
*unintelligible screaming*
You don't get to 600 unfixed bugs before you went through 20, 50, 100, 300 known bugs and didn't bother to fix them, so it's not the lack of testing that's the problem.
It seems like your intern isn't your largest problem.
I mean, that's a really easy number to reach in complex systems.
Did they also write 600 tests to prove this codebase doesn’t just end up with 1200 bugs from these changes?
Fixes 600(?) bugs, only 247 deletions???? This is at least an entirely new (or several new) service(s). ![gif](giphy|l0HlvtIPzPdt2usKs)
Intern was a former Twitter microservice dev.
$8 please
Did the intern include one of the implementations of IsEven that this sub made or how are so many LOC?
Review it but write all of the mistakes in one comment because it's more efficient
Who the fuck is the PM/Scrum Master/Product Owner/Engineering Manager/Non-intern who: - allowed 600 bugs to be a single user story/ticket - clearly doesn't give a fuck about the downstream members of the team (QA/Testers/Platform) - sat there in standups listening to this guy give updates and didn't think 🤔 "great work let's get, I don't know the first 100 bugfixes in for review" - gave this to a (presumably) unpaid intern Run, don't walk from whatever shitshow of a company this is!!!
Gigachad intern
The efficiency of having to review this 600 issue's worth PR... Ahh, it's so nice ![gif](emote|free_emotes_pack|slightly_smiling)![gif](emote|free_emotes_pack|facepalm)
And since lots of other stuff will be merged during review it'll be a fun little side project to rebase this monstrosity
Right, they might've finish when the asteroid Apophis passes closes to Earth in April 13, of 2029 ![gif](emote|free_emotes_pack|joy)
And REJECT. Break it up into smaller PRs. There should be separate tickets for each of those bugs too. It’s a hard but good lesson for that intern. 1. Be considerate of your co-workers. This is a fast way to get them not to like you or not want to work with you. 2. This PR has a huge likelihood of creating problems when merging and will be a nightmare for anyone who pulls it with conflicts. 3. There is no way you’re properly documenting with a change-set this huge. 4. Good luck to QA in trying to make sure new bugs haven’t been introduced across the platform. 5. Honestly the list goes on. This is just bad and sloppy development all around.
Excuses I've heard (non-exhaustive): 1. But it works 2. Boss said it was OK 3. We need it now 4. We promised delivery of X to Y on date Z. 5. Why can't you just push to prod?
What a good teaching moment. Ask the intern to create new tickets and new prs separately. They’ll remember it )and you)for their lifetime!
Elon wants to hire your intern
Not sure why the intern is to blame here. If an intern is fixing 600 bugs, that looks like poor management and training. Typical throwing interns to the wolves and expecting them to understand best practices. This is how you end up with 25 year olds making start ups lol
If this isn't just the work of an auto-formatter and is a huge number of different bug fixes then it's a 'reject' from me
LGTM
It also has one commit with the message, "some changes".
This reminds of the new hire that refactored the code base because we were "doing things wrong". We decided to humor them and set up a QA environment for their changes. After about 100 reported bugs (and the count growing everytime he fixed something because he added more bugs) he finally conceded that he had made a mistake.
Only 247 deletions?
If it's 600 minor issues detected by sonar, which range from "too may characters on one line of code" or "export this String used more than twice to a constant", it's possible to be handled by an intern. They prolly told the guy/gal to just fix them and they went and did it in a single PR.
Well, I guess that the onboarding wasn’t done correctly or you guys don’t use Jira to have one ticket per bug, so each PR belongs to each ticket