Generally I'm sanity checking. Does this look like code? Are there any obvious typos? Any residual debugging statements? Anything that feels like a mistake?
If I want to get more involved, I'll pull the branch and test it. But sometimes it's better to get the code merged and fix bugs later, in smaller PRs.
The longer a pull request is, the less likely the reviewer will catch anything of importance.
It started as nice javascript in the 1st picture and in the 2nd is just a clusterfuck of symbols and letters hahaha. I agree with the smaller PRs, easier to catch stuff there.
That's a great point, and to clarify, I wouldn't usually approve a PR with bugs. But I think there are cases where it's acceptable:
The only bugs are design inconsistencies or unclear requirements (in larger organizations, these can take weeks to resolve).
We're doing new feature development, the PR is a major net improvement over what we had before, and the bugs are minor and part of the new feature (not regressions).
The team agrees that the PR is large enough and affects enough things that we should merge, deploy it to the company's internal server, and get a lot of eyes on it as quickly as possible. This is an intentional delay of all other work until the code in question is thoroughly tested.
Every day that a PR sits unresolved is technical debt; it gets further and further out of sync with the master branch, the original author starts to forget what they did, and all the people that could have been using and testing the new code, aren't. So the risk of letting bugs into production isn't the only risk in this situation, nor, I submit, is it always the greatest risk.
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
Generally I'm sanity checking. Does this look like code? Are there any obvious typos? Any residual debugging statements? Anything that feels like a mistake?
If I want to get more involved, I'll pull the branch and test it. But sometimes it's better to get the code merged and fix bugs later, in smaller PRs.
The longer a pull request is, the less likely the reviewer will catch anything of importance.
It started as nice javascript in the 1st picture and in the 2nd is just a clusterfuck of symbols and letters hahaha. I agree with the smaller PRs, easier to catch stuff there.
I would advise against merging code with bugs, which can end up in production pretty easily if you are not careful.
That's a great point, and to clarify, I wouldn't usually approve a PR with bugs. But I think there are cases where it's acceptable:
Every day that a PR sits unresolved is technical debt; it gets further and further out of sync with the master branch, the original author starts to forget what they did, and all the people that could have been using and testing the new code, aren't. So the risk of letting bugs into production isn't the only risk in this situation, nor, I submit, is it always the greatest risk.