When hiring developers, have the candidate read existing code • Hacker News Board

Re: When hiring developers, have the candidate read existing code

#451

Post by boomlinde » Sat, Apr 16, 2022, 3:30 PM UTC

doix wrote: post #368

boomlinde wrote: post #289

Earlier quoted context omitted.

Where I work, development is trunk based and without pull requests, and all code is reviewed. When I want to submit code, I push it to a staging area that tracks master. This causes the commits to appear in Gerrit where (conflict-free) rebasing can be performed with a single button and the code can be reviewed. During the staging I can change anything about the commits to my hearts content. Once everyone is satisfied…

What you are describing, is pretty much exactly how it works at every company I've worked at that used PRs. You create a new branch from master, make your changes, push the branch to github/bitbucket/gitlab, make a PR. While the PR is open, you can make any changes you want to the commits in that branch (since it's just a branch). People look at the PR (which shows the diff) and approve it if they are happy, or reque…

I've used that workflow before, but it was called "merge requests", presumably because you're not actually requesting anyone to pull from a remote, but requesting someone to merge one branch into another.

Re: When hiring developers, have the candidate read existing code

#452

Post by zeroonetwothree » Sat, Apr 16, 2022, 3:36 PM UTC

michaelrhansen wrote: post #25

This is my standard practice - I give a real world data fix script that has been simplified to understand and comment on in 20 minutes. The interviewee is the code reviewer before it gets run in production. It covers things like performance, security, typical syntax mistakes, and working with sensitive data. The best part is actually not the script, but the stories it triggers about past challenges.

I think it’s fine to have this as part of your interview but I would be worried to have it as the only thing.

Re: When hiring developers, have the candidate read existing code

#453

Post by boomlinde » Sat, Apr 16, 2022, 3:38 PM UTC

zo1 wrote: post #389

boomlinde wrote: post #289

Earlier quoted context omitted.

Where I work, development is trunk based and without pull requests, and all code is reviewed. When I want to submit code, I push it to a staging area that tracks master. This causes the commits to appear in Gerrit where (conflict-free) rebasing can be performed with a single button and the code can be reviewed. During the staging I can change anything about the commits to my hearts content. Once everyone is satisfied…

I don't understand at all what you're saying. Sounds like you push your code to a branch called master that tracks master but sits on a separate repo and then you apply your commits after approval via patches? Seems like you're recreating the concept of branches using repos and then using patches instead of merging. Either way, just so you know, this kind of attitude and that you think it's a-ok to use such a convolu…

I'm not sure how you read what you're suggesting into my description. I make a commit.

    git commit
    # ...
    git commit # or two

I push them to the staging area for a given branch, in this case master

    git push HEAD:refs/for/master

This isn't a branch in the traditional sense but git conveniently sees it that way.

Now they're immediately available for review. That looks something like this (this is not from my workplace, but the Go team's Gerrit instance): https://go-review.googlesource.com/c/go/+/361916

> Either way, just so you know, this kind of attitude and that you think it's a-ok to use such a convoluted process for what is effectively the same thing would 100% mean me not hiring you.

That's fine. There are plenty of companies that manage to hire based on competence, experience and references so I'm not exactly aching to get hired at a place that would deny me for having used Gerrit.

Re: When hiring developers, have the candidate read existing code

#454

Post by jsiaajdsdaa » Sat, Apr 16, 2022, 3:49 PM UTC

csmpltn wrote: post #420

buf wrote: post #382

Earlier quoted context omitted.

Hi, comment OP here. I decided to blog about this. It's cheeky, apologies. https://siliconvict.com/articles/6-how-to-hire-actually-good...

You do you. I can't see how your approach can scale to hire tens or maybe hundreds of engineers. Sounds like something that may somewhat work for you when you're the only one doing the hiring for your small team of buddies at a startup somewhere. You're not even trying to tackle things like hiring for multiple roles, interview fairness, subconscious bias, technical diversity, feedback, or making the interview process…

If you need hundreds of engineers to solve a related set of business problems[1], I wager you've done something wrong.

[1] and if these problems are unrelated (think Uber's human driver app vs self driving), then why are candidates passing through the same committee?

Re: When hiring developers, have the candidate read existing code

#455

Post by wqweto » Sat, Apr 16, 2022, 3:54 PM UTC

buf wrote: post #382

jahnu wrote: post #270

Earlier quoted context omitted.

Surely this will filter out ~50% of people who are good but don’t have any public code? I have a family and as such no free time for coding so I haven’t written any code I can legally show anyone else in more than a decade. But everyone who has employed me is more than happy with my work. Not to mention code is only half of why you would want to employ any developer.

Hi, comment OP here. I decided to blog about this. It's cheeky, apologies. https://siliconvict.com/articles/6-how-to-hire-actually-good...

Very useful!

Posted this on behalf of the silent majority that likes the approach.

Re: When hiring developers, have the candidate read existing code

#456

Post by buf » Sat, Apr 16, 2022, 4:03 PM UTC

BlargMcLarg wrote: post #444

buf wrote: post #382

Earlier quoted context omitted.

Hi, comment OP here. I decided to blog about this. It's cheeky, apologies. https://siliconvict.com/articles/6-how-to-hire-actually-good...

Entire post can be summed up as "this is my answer, here is me rationalizing correlation=causation without any empirical evidence". We could dissect the entire thing but really, if that is how it works for you, fine. Just don't push it onto others as the absolute truth. If this stuff was really so great, empirical research would've hammered it home decades ago. But it doesn't, and continues to struggle finding any me…

This isn't used widely because it is hard. But it's not but novel.

1. Companies want to standardize their hiring when really they should be looking to customize it to each candidate.

2. Lots of companies want to spread the blame of a bad hire across a committee of 4 or 5 people, but I think if you looked you'll find many startups doing it this way. They stop when they grow to a large size. They do it because frankly hiring good engineers is no longer the top priority (it's typically quotas AKA butts in seats).

Re: When hiring developers, have the candidate read existing code

#457

Post by isbvhodnvemrwvn » Sat, Apr 16, 2022, 4:04 PM UTC

enraged_camel wrote: post #176

twblalock wrote: post #171

Earlier quoted context omitted.

Don't do it unless you are going to treat keeping comments up to date the same way you treat keeping the code up to date. I've lost count of the number of times I've seen comments that were just wrong -- they applied to the code as it was written years ago, not today. Write good code, and it should be obvious how it works. Use comments sparingly and only when things are not obvious. Update the comments when you chang…

>> Don't do it unless you are going to treat keeping comments up to date the same way you treat keeping the code up to date. This should be easy enough to verify during code review, no?

Only if the comment is very local, and not e.g. two calls up the stack.

Re: When hiring developers, have the candidate read existing code

#458

Post by doix » Sat, Apr 16, 2022, 4:07 PM UTC

boomlinde wrote: post #451

doix wrote: post #368

Earlier quoted context omitted.

What you are describing, is pretty much exactly how it works at every company I've worked at that used PRs. You create a new branch from master, make your changes, push the branch to github/bitbucket/gitlab, make a PR. While the PR is open, you can make any changes you want to the commits in that branch (since it's just a branch). People look at the PR (which shows the diff) and approve it if they are happy, or reque…

I've used that workflow before, but it was called "merge requests", presumably because you're not actually requesting anyone to pull from a remote, but requesting someone to merge one branch into another.

Yeah, it's one of those things where the term everyone uses strayed from the etymology. Probably because in the GitHub UI it still calls this a pull request despite not pulling from a different remote.

Either way, if you read the thread, the folks are arguing that giving someone the power to block code getting into the mainline is bad. Judging by what you described, where someone has to approve before it can be merged, we both agree to disagree with them. We are just arguing the semantics of how that is implemented.

Re: When hiring developers, have the candidate read existing code

#459

Post by jansommer » Sat, Apr 16, 2022, 4:08 PM UTC

zeroonetwothree wrote: post #449

amai wrote: post #409

In my company we go one step further: We encourage the candidates to „bring their own code“. Then we let them explain their code, discuss possible issues, extensions and so on. Usually simply from looking at the code style one can infer a lot about the candidates level of skills. Also the candidates are less nervous and are sometimes really enthusiastic explaining their favorite side project. All in all it leads to a…

How does this work if they only have coded as part of jobs? Do you assume everyone is working on some side project or open source?

Would make sense to provide it as an option, and approach candidates without side projects in a different way. I would much rather show up with code I've already written and use that as a starting point for conversation.

Re: When hiring developers, have the candidate read existing code

#460

Post by mdavidn » Sat, Apr 16, 2022, 4:11 PM UTC

NateEag wrote: post #396

someweirdperson wrote: post #286

Earlier quoted context omitted.

Explanations should end up in comments, documents, and the commit-comment of the squash (creating a good one takes some time when squashing). The PR and original branch show dead-ends that should not be required reading to understand things.

I'm personally a fan of small, clean commits, rebasing without squashing before merge, always making a merge commit, and writing a clear, through merge commit explaining what the branch does. That way, I can treat the series of merge commits to trunk as the simple, linear overview of history, but when I'm bug-hunting I get small, clear commits to search through with git bisect. It also means I get more useful blame o…

I'm with Nate here. Commits are a form of documentation and can be useful for grouping together related changes. All of this context is lost when squash merging. That said, I do aggressively rebase and amend commits on the feature branch to consolidate commits into one for each change, including any minor fixes discovered later.

For example:

When I want to see tests or documentation or config related to a change, I'll find the commit and look for other lines changed at the same time.

When I make automated changes to the code, like reformats, auto-corrections by a linter, or IDE refactors, I create a separate commit to separate mechanical changes from those that require more human scrutiny.