6 ms·
When I was interviewing GitHub employees for my book (https://buildingtoolswithgithub.teddyhyde.io/ https://buildingtoolswithgithub.teddyhyde.io/) one of them m
by xrd 8y ago
When I was interviewing GitHub employees for my book (https://buildingtoolswithgithub.teddyhyde.io/ https://buildingtoolswithgithub.teddyhyde.io/) one of them made a surprising comment. Early on, his GitHub teammates told him he should create an empty PR with a plan of attack before writing any code. I thought it was a brilliant idea and most teams don't do this. Smart of them to make this a separate feature instead of just bolting on WIP.
- darkerside 8y agoIs there actually a way to do this, if there is no code difference between your new branch and the base?
- NickBusey 8y agoWell I personally like to start by writing the documentation first. That way your documentation changes become essentially your 'plan of attack' rather than having to write it out explicitly.
- sjburt 8y agoIn my team we'll just create a README or TODO and make a PR with that.
- nmjohn 8y agoIIRC, you just need a commit different in order to open a PR with github. To make an empty commit: git commit --allow-empty -m 'Empty commit so I can open a PR'
- darkerside 8y agoI thought Github looked for an actual code difference in the PR creation process, but it seems you're correct! Thanks for the tip.
- jolmg 8y agoIs that even necessary? I would have thought that simply making the branch and pushing it while it's on the same commit as the branch you're branching from would be enough.
- MapleWalnut 8y agoYou can't merge a branch that is even with another.
- jolmg 8y agoWell, sites like GitHub could be designed so the PR appears as merged from the beginning and then enabling the "merge" button, once more commits are available. Doing it this way also allows to naturally support merging a PR/MR more than once, which I don't know if it's undesirable, but it more closely aligns with what git supports.
- mikekchar 8y agoThe way I've done this before is to have a directory in the project called TODO. In the directory, I add a file with the same name as the feature branch I'm going to work on. I put a description of the work I'm going to do (and any TODOs if I think of them). I commit that and submit a PR with the title "DO NOT MERGE -- whatever". I find that nobody looks at them, though. :-)
- deckar01 8y agoMy team recently started requiring an implementation proposal on the issue that has to be approved by their peers. It saves a lot of wasted effort by exploring the problem domain collaboratively before development begins.
- manyxcxi 8y agoI’ve seen people suggest this and then the horror on other engineers faces that follows. In my opinion, the difference between a good eng/dev and a great one is that they recognize when the effort is too big to chew in one bite, too unknown to reason about clearly without a plan, and/or too many gotchas already exist in the codebase to make it easy. The great developers I’ve known would pull back and think about the problem longer, draft up some plans and noodle on it with colleagues and I love that foresight. The problem with a policy like this is that you’re slowing everything down for the sake of the few that might need it. Your team leads should be able to determine which ones should require the investigation and keep the team moving efficiently.
- deckar01 8y agoIt shouldn't be slowing good developers down, it should just be changing the order they were already doing things. One of the most fundamental ideas I took away from my Software Engineering course in college was that catching errors early in the development process is more efficient than catching them later. It is a lot faster to grind out ideas in the planning phase than to churn during code review, because the development and code review processes are a lot slower than just replying to comments. More importantly, when experienced developers formalize their reasoning for a proposal, less experienced developers can learn from it. That same knowledge becomes archived in a format that is easy to discover and consume in the future.
- aviraldg 8y agoOne of my previous teams used to do something similar, but the initial PR would be for a change to the docs for the project. Would be great, because the docs would be updated to reflect any new changes automatically.
- DogLover_ 8y agoSomeone suggested this on my team. I personally don’t like the idea because these policies often times lead to bureaucracy and then nothing getting released. It is not that I am against thinking ahead but if i have to in details explain everything I do, then more time is spent documenting than actually creating which is the part I enjoy.
- WrtCdEvrydy 8y agoWe tend to split workloads among teams based on timezones, so using placeholder PRs are required if you're handing your ticket over to another timezone for the night.
- astazangasta 8y agoIf your workplace is optimizing for your enjoyment, there is probably a serious problem afoot. The value of thinking ahead, to be clear, is that it enables you to "actually create" something that isn't ill thought out, or worse, of no actual use to any person. It also allows your work to be understood by other people, who might have to duplicate it, review it, or extend it in the future. It is indeed unfortunate that all of these legitimate demands should intrude on your time that could instead be spent spewing out code.
- DogLover_ 8y agoHave to disagree. Many places try to optimize for enjoyment so they can keep their employees. There is a time and place for everything. If you a working on a project that takes months it should be planned ahead but if it only takes a few days you might not need. Let us not forget why we got rid of waterfall model.
- Udik 8y ago> Let us not forget why we got rid of waterfall model. It's actually doubtful it ever existed: its first formal description, in the 1970s, is used as an example of a model that doesn't work. Let me formulate the Godwin-Udik's law: "As an online discussion on development practices grows longer, the probability of a mention of waterfall as a negative example approaches 1"
- nawitus 8y agoWhy not add the plan of attack to the issue in the issue tracker?
- kjeetgill 8y agoI think this feature is meant to be a good middle ground rather than having one issue and one PR ticket. DO you want an issue with added patch set power, or a PR with better collaboration? They'd look pretty similar either way. They went with the latter.
- SergeAx 8y agoIt makes sense to separate what we want to achieve and how we gonna do it. You may actually have several WIP MRs for one ticket.
- notatoad 8y agoIn my experience, communicating too much in the issue tracker tends to invite unnecessary discussion/bikeshedding with people who don't need to know the plan of attack. Our support staff are on our issue tracker, but not on our PRs. Stuff in the PRs is developers-only.
- arcticbull 8y agoIndeed, code review shouldn't be architecture review. At that point it's too late and the friction/cost of revisiting the structure is often high. Early sign-off on broad strategy is great for productivity even if it takes a while to document the approach ahead of time.
- mikekchar 8y agoBack when I was first doing XP style development, we did everything with trunk based development (TBD). As a rule, we pushed to trunk (master) every 20 minutes or so. Since we were all co-located (i.e. in the same time zone and working relatively similar hours) it meant that there would be a push to trunk every 5 minutes or so (frequency depended on the team size, of course). We read all of the diffs between trunk and our branch before we merged our code -- and integrated with any changes. Probably most people won't realise, but it was in that kind of situation that "continuous integration servers" came into being. The idea was that after you pushed to trunk, the tests would run (and in those days we slapped your fingers if the test suite ran for more than a minute or two -- for reasons which will become obvious) and you would have a minute or two to fix any gaffs you made merging conflicts, etc. If you wanted to merge your code, you would wait until the CI server gave a green light, so if the tests were slow, it would be a major PITA. The massive upside of this process was that you always knew what your colleagues were planning to do. You saw all their progress every 20 minutes! You integrated with their changes! If you saw a problem, you yelled over the cubicle wall and said, "Hey that won't work because I'm doing X" -- saving both you and your colleague hours of wasted effort. TBD in this style is hard, but very rewarding. The main reason I don't practice it these days is because 1) We have continuous deployment and you don't want to deploy unfinished work 2) The teams I work on these days are distributed (and I'm 9 timezone hours away from their day). However, I really miss that immediate feedback and the seemingly effortless way in which everyone on the team is kept completely in sync. I've never found anything that even comes close (and I've done the "preliminary PR" thing many times).
- Scarblac 8y agoThat was with a very small team? Seems that if you get to only five people or so you'd already be spending most of your time understanding their commits.
- lazerwalker 8y agoIn my experience, not so much? You'll need to worry if you're working on tasks that touch the same corner of the codebase, but larger teams tend to correlate with larger codebases, meaning that's not necessarily more likely. That sort of XP style development also tends to come hand-in-hand with near-constant pair programming. So even if the conceptual limit is "5 sets of committers", that's still double the actual team size. And in practice, the cross-pollination that comes with swapping pairs every day or every few days means there's less of a cognitive load to needing to be aware of what's going on, since chances are either you or your pair already has context.
- Waterluvian 8y agoWe do this as a design document on a wiki. It's more long lasting that way and richer with better text and media support.