Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Why I close pull requests

jeffgeerling.com

Post reply on HN
Print view View original

61–70 of 188 posts

Re: Why I close pull requests

#61

Post by viraptor »

fdsfsaa wrote: post #47
yegle wrote: post #13

At Google, if you want to implement new features (or large refactoring), you'll need to write a design doc. In which, you should answer questions your reviewers might ask (common questions like: why do you want to do this, what are the alternatives, how components interactive with each other before/after your change). This is something like Python's PEP: you need a proposal to convince your reviewer that you have put…

Requiring permission to do work is the enemy of progress and engineering dignity. It creates a presumption of incompetence and an atmosphere of low trust that punishes people who want to push the envelope of what's possible. Google's design document culture is bad. Google has succeeded in spite of it. In my experience, having worked at many large tech companies, design documents obfuscate, not enlighten. They become…

I think it really depends on the project, the community, etc. Some projects seem to be quite successful with design documents. (go, python) Some make it the process where changes go to die unless you're part of the core team and can poke the right people personally to review them. (openstack) The culture is everything.

But I think you're missing some very practical things here:

> Why are you making this change? What impact will it have? --- can be asked during review of actual code.

Yes, and they're going to be asked every single time. And if the reviewers disagree with the impact, you'll have to either drop the change or rewrite it. So why not ask first?

> There is no need to build a speedbump, not if you trust your people.

"your people" works on a small scale with people you work with continuously. It doesn't work on the internet when someone called "fdsfsaa" submits the code to your project and you hear of them for the first time.

> Most changes don't need this process.

Depends on the stage of development goals, etc. Some projects will get bugfixes and maintenance. Some will be constantly changing. You can't generalise.

Re: Why I close pull requests

#62

Post by [deleted] »

fdsfsaa wrote: post #47
yegle wrote: post #13

At Google, if you want to implement new features (or large refactoring), you'll need to write a design doc. In which, you should answer questions your reviewers might ask (common questions like: why do you want to do this, what are the alternatives, how components interactive with each other before/after your change). This is something like Python's PEP: you need a proposal to convince your reviewer that you have put…

Requiring permission to do work is the enemy of progress and engineering dignity. It creates a presumption of incompetence and an atmosphere of low trust that punishes people who want to push the envelope of what's possible. Google's design document culture is bad. Google has succeeded in spite of it. In my experience, having worked at many large tech companies, design documents obfuscate, not enlighten. They become…

[deleted]

Re: Why I close pull requests

#63

Post by mikeash »

tedunangst wrote: post #28
mikeash wrote: post #20

Earlier quoted context omitted.

I've also seen people who don't understand how his stuff works, and fork projects before cloning them because they think that's the process. A relatively small number of clueless people could result in a lot of pointless forks.

The big button does say "fork me on github" after all.

Indeed, if you're not quite sure what to do, then that would be an obvious one to try. And it'll work in the end, so you won't necessarily change afterwards.

Re: Why I close pull requests

#64

Post by teacup50 »

fdsfsaa wrote: post #58
zardeh wrote: post #56

Earlier quoted context omitted.

>Requiring permission to do work is the enemy of progress and engineering dignity. It creates a presumption of incompetence and an atmosphere of low trust that punishes people who want to push the envelope of what's possible. Its not a permission to do work, its permission to merge the results of the work to master. Those are different. >Google's design document culture is bad. Then why do we have JEPs, PEPs, and rfc…

Counterexample: the Linux kernel.

Which has succeeded in spite of being a hot, churning mess.

Re: Why I close pull requests

#65

Post by edmundhuber »

fdsfsaa wrote: post #47
yegle wrote: post #13

At Google, if you want to implement new features (or large refactoring), you'll need to write a design doc. In which, you should answer questions your reviewers might ask (common questions like: why do you want to do this, what are the alternatives, how components interactive with each other before/after your change). This is something like Python's PEP: you need a proposal to convince your reviewer that you have put…

Requiring permission to do work is the enemy of progress and engineering dignity. It creates a presumption of incompetence and an atmosphere of low trust that punishes people who want to push the envelope of what's possible. Google's design document culture is bad. Google has succeeded in spite of it. In my experience, having worked at many large tech companies, design documents obfuscate, not enlighten. They become…

At a large Internet Company that I worked at, my experience was (seemingly?) quite unlike yours.

While writing a CEP, my gut reaction was like yours, "this is a waste of time, code is art, and I am programmer Picasso," etc. But since my manager and my technical leads at the time were very good, I bit my tongue and did the work of writing up a CEP anyway. In that environment, it didn't take that long to write a CEP. It was about 2 pages. And since I was proposing to change how a critical piece of infrastructure worked, it was really important for the oncall people to be on board, and it had to make sense, and it was important to identify all the failure points in advance, etc. From the egocentric perspective, I'm certain that it was much less work for me to write those 2 pages than it would have been to explain to 20 different people what I'm doing and why, either at the water cooler or during code review.

Trust is often earned and not given. Your coworkers may not know you or the quality of your work. Under the right conditions, I don't think being asked to write a CEP is being asked to dilute your vision, it's merely being asked to define it, and describe how it fits in with how things already work. If that antithetical to you, then you need to work at or found a small company, where everyone is most concerned (hopefully) with making something work, rather than trying to make something that is already working and already making money better.

I have some points of agreement with you, although I wholeheartedly disagree with the conclusions you make.

"Everyone knows that in practice, nobody updates design documents after the fact." Even if this is completely true (and it isn't -- I have written and read many documents that closely follow current practice) does that make it better to not try?

"Sometimes the worth of code isn't apparent until it's made." Does this mean that it is too hard to explain why it's worth doing?

"These questions you highlight --- Why are you making this change? What impact will it have? --- can be asked during review of actual code." You're right, these questions will certainly be asked -- in which case, you can link them to the CEP which will probably answer their questions plus other ones that they didn't even think of to ask.

I can't disagree more that code review can replace a CEP.

Re: Why I close pull requests

#66

Post by akkartik »

mikepurvis wrote: post #37
dclowd9901 wrote: post #23

You know what would be cool? If I could create a fork off the project i was using. Then I write a feature I need in that project. it becomes a PR, but then the fork is also automatically (if possible) updated whenever the main branch is updated. If it can't be updated automatically you are notified to update your fork against the upstream changes. This would have many benefits, including easy testing of PRs, forks th…

I don't know about "automatically", since all the hashes would change, but I definitely do wish that Github had better tools for managing long term forks, especially web workflows for: - rebase this branch on upstream - resolve merge/rebase conflicts - make my master be upstream's master with PRs X, Y, and Z applied to it in that order. I find the last one in particular to be awful, where I'm iterating on multiple PR…

Have you considered just constantly rebasing as you pull? This works well for private forks, but it may also be doable for public forks if you explain what's going on in the Readme. Just an idea. I know it goes against everything we learn about Git, but perhaps it's not so bad in this particular situation.

Re: Why I close pull requests

#67

Post by nathancahill »

yegle wrote: post #13

At Google, if you want to implement new features (or large refactoring), you'll need to write a design doc. In which, you should answer questions your reviewers might ask (common questions like: why do you want to do this, what are the alternatives, how components interactive with each other before/after your change). This is something like Python's PEP: you need a proposal to convince your reviewer that you have put…

As a less formal version of PEP/design docs, I always open an issue on Github projects proposing the changes that I would submit in a PR before I do the work, and end by asking if there's interest in a PR that implements those changes. That requires very little effort and avoids a lot of wasted time on both sides.

Re: Why I close pull requests

#68

Post by ageofwant »

mikeash wrote: post #44
ageofwant wrote: post #41

Earlier quoted context omitted.

I sometimes use the fork button just to get a copy I may or may not play with over the weekend.

How does that help? Are you just using the presence of the fork in your account as a sort of bookmark?

Yes, a complete and fully featured bookmark that won't disappear. It costs me absolutely nothing, and is ready to go when and if I need it. Why wouldn't I do that if I could ?

Re: Why I close pull requests

#69

Post by zardeh »

fdsfsaa wrote: post #58
zardeh wrote: post #56

Earlier quoted context omitted.

>Requiring permission to do work is the enemy of progress and engineering dignity. It creates a presumption of incompetence and an atmosphere of low trust that punishes people who want to push the envelope of what's possible. Its not a permission to do work, its permission to merge the results of the work to master. Those are different. >Google's design document culture is bad. Then why do we have JEPs, PEPs, and rfc…

Counterexample: the Linux kernel.

Which leads to problems: see systemd.

Also, to be clear, Linux is relatively small compared to Google or Microsoft, or indeed many corporations codebases. Someone could conceivably read the entire Linux kernel codebase. That's not true for BigCorp.

Re: Why I close pull requests

#70

Post by zardeh »

fdsfsaa wrote: post #55
stinkytaco wrote: post #50

Earlier quoted context omitted.

I can't comment on Google's culture, but: >Design documents turn every feature into a half-assed, lowest-common-denominator risk-minimized shell of itself. Sounds like a sentence written by someone who is an engineer and not a support staff or a user, i.e the people who have to deal with the fallout of every feature change and every engineering decision. I could just as easily substitute "feature driven design" into…

> Sounds like a sentence written by someone who is an engineer and not a support staff or a user Do support staff and users sign off on design documents? "No" is the universal answer. Are you claiming that engineers aren't reasonable human beings who can take support staff and user concerns into account on their own? What makes you think the people reviewing design documents can do that? Is it that you just trust a s…

>You shouldn't hire people who don't give enough of a shit to take the big picture into account.

At a certain point you can't. I was recently asked to implement a feature for a usecase for another engineer. It required a design doc and review by a few representatives from related teams. He and I wrote the doc, and the initial review was that any solution that would fix his usecase would break the testing infrastructure for practically every other developer in the building. He could write a few fewer loc when testing, at the cost of tests failing due to unrelated changes.

Neither he nor I had the awareness of the impact, and realistically couldn't have, it was neither of our responsibilities. And because I spent an hour filling out a template document, I didn't need to waste my time doing that.

Post reply on HN