4 ms·
I could be wrong, but I feel this actually enables the wrong kind of behavior we should expect from developers. We want fast, continuously integrated code that
by royosherove 8y ago
I could be wrong, but I feel this actually enables the wrong kind of behavior we should expect from developers. We want fast, continuously integrated code that is merged early.
Usually that means using feature toggles so that we can see compilation issues without the code being active by default.
This feature signals "yeah it's OK to take your time and not merge stuff incrementally - here's a feature just for that since it's such an important part of your workflow".
Yes, people have been doing it anyway but at least it felt a bit like a hack, like it's not supposed to work that way.
Am I crazy, or is this pushing back progress made in CI coding practices?
- chrisseaton 8y ago> We want fast, continuously integrated code that is merged early For some projects maybe. Other projects want changes to be very carefully presented, considered and reviewed. > it's not supposed to work that way According to who? Is this how you do things at work? Ok, but other people work differently. There’s not authority in programming to set what we’re ‘supposed’ to do. > is this pushing back progress made in CI coding practices? Seems orthogonal to me. You can run CI on a branch. But overall, no not everyone’s development is a mad dash to get everything merged instantly and some want carefully created PRs.
- deleted 8y ago[deleted]
- benjiweber 8y ago> Seems orthogonal to me. You can run CI on a branch. Funny how some terms can be diffused enough over time to come to mean almost the opposite of what they originally meant. Continuous Integration is about integrating continuously. "the practice of merging all developer working copies to a shared mainline several times a day." i.e. everyone commits to master multiple times a day. Then tooling gets built to facilitate this way of working. People start confusing build tooling with CI the practice. Eventually that tooling gets used to support people working in continual isolation on feature branches. People start calling this practice CI even though there's no integration in sight. Of course true CI isn't applicable for all contexts. Draft pull requests look like a great tool for where CI is impractical.
- chrisseaton 8y agoMerging into a single branch isn’t the definition that people like ThoughtWorks seem to use. They just talk about integrating into a shared respository, not specifically a shared branch. The CI system then does the work for you to test that on top of master. You’re talking about developers doing things that the CI system is there to do.
- gunnarmorling 8y agoCI only seems to make sense to me when talking about a single branch ("master", "trunk") into which all changes are frequently merged. FWIW, also Wikipedia describes it as "merging all developer working copies to a shared mainline", the latter being a branch, not repo. Otherwise, you still might test indvidual feature branches on top of master, but that wouldn't prevent any conflicts between several of the pending branches. One could think of a system that continuously tests all possible combinations of all pending feature branches to spot any conflicts before actually integrating them. Not sure whether such thing exists, though, or whether it'd be very practical.
- kjeetgill 8y agoIt really does depend on what you're developing and what you're shipping. Even minutia like language and build systems influence what's a convenient way to manage in progress work. I'm much more tolerant of dead-code/feature flag branches in certain parts of my codebase than others even. Sometimes you know exactly what you're building, but other times you're trying to sketch out an idea to see what it might look like and to get buy-in for a feature. You'll need to decide if you want to put the drag of partial work on everyone else who's making changes or on yourself to keep up with HEAD. It's a choice. All that said I think this feature in github is minor enough that it'll make what people were doing already more convenient (UX win) but won't substantially change how people work. It's just not dramatic enough.
- royosherove 8y agoYeah I can see that. Maybe the term "PR" can lead to misuse with things that would be better off integrated. I wonder, Perhaps a better name that does not imply that use case might make better sense.
- danpalmer 8y agoI often write just my database models, or just my API/schema first. It's typically syntactically correct, but entirely non-functional. I'll sometimes open a PR for this with "WIP" in the title, "DO NOT MERGE" in the body, or without the "For review" label that we use. These PRs serve to facilitate discussion, start knowledge sharing, and update other developers on my progress more clearly than a branch they might have to check more carefully. However I would definitely not want to merge one. I love CI/CD, we do a lot of it and try to have small changes and feature flagging as much as possible, but this feels pretty useful in general for us. Yes it could be mis-used, but I don't think that's a reason not to have the feature.
- royosherove 8y agoI can see how that makes sense. Thanks for explaining!