Using Bots to Solve Developer Drudgery
kensodev.com
kensodev.com
If a bot "approves" a PR in this fashion, it's not an approval, you've just set up a "changes to this file are not worth reviewing" rule, and handed out a green check-mark particpation award. Just let your developers force-merge such non-issue pull requests. The very essence of PR approvals is someone else looking at your masterpiece, thinking about it and pointing out the things you forgot. If you don't have time for that, be honest with yourself and let go of the mandatory approval requirement.
As for the latter part, the instant PRs and merges for updates, congratulations on re-implementing a monolith while calling it a microservice.
Sincerely, - A very cranky developer currently very slowly steering a carelessly reviewed codebase back on track.
However, the JSON definitions are just not that important, that's a fact, we can ping engineers and have them lose focus just to get approve or we can decide to automatically approve if it's under directory "X".
I would argue that we did not implement a monolith at all, we just have a faster cadence to update dependency than most.
With the way this works, you can't forget anything and this is why this automation is so powerful, if you change the SDK, you know that everything using this SDK is updated with the latest code. It will either fail tests immediately or you can just move on with your life and focus on more important things
Some relevant context: - Despite your interpretation, we believe strongly in PR review outside of this narrow area. In our business, security and appropriate controls are a $BIG_DEAL and being able to say that every PR goes through a well-defined approval process (even if part of that process is automated) carries infinitely more weight than "developers force push whenever they feel like it." - One of the benefits of auto-approval is that developers control when these kinds of changes are integrated. We thought about having a fully-automated process where changes in services automatically update this library and decided that this behavior wasn't desirable because many API changes need to be _coordinated_ with other changes. We want developers to be able to decide when to merge this kind of PR in conjuction with other PRs. (The alternative is a strong commitment to backwards compatibility at all times, which is not a constraint we're willing to impose on every developer at this time in our business. YMMV.) - I honestly don't understand your comment around re-implementing a monolith, but I'd love to talk about it more since this is a topic I have many opinions about. My take is that micro-services should interoperate via HTTP APIs and that these APIs should be automated as much as possible. We do this by having our code generate Open API (aka Swagger) definitions based on our code and publishing the resulting schema at `/api/v[N]/swagger` in a deployed service. However, we still need a way for other services to to integrate with these APIs. So far, a "monolith" client library, updated in the way this post describes, has worked quite well. I am eager to hear about alternatives.
it implicates that the developer who wrote the code, controls when it gets merged without a "proper" review
of course i'm unaware of your codebase and your entire workflow, but a change / addition in a swagger file implicates a change in an endpoint of your api. how do you make sure other applications / resources with the changed library as a dependency, don't run into problems when you're auto merging and auto bumping dependencies?
If the swagger files are being autogenerated, then why would you ever review them? You should test them, but reviewing isn't appropriate. You should review the things that generate them instead. When they change, that's a build artifact (I don't review my generated libraries and binaries, I only test them before delivering them).
On the commitbot, I hope you're not doing that in deployment without running it through test first. Can it also generate issues if a particular service fails its tests after the update?
commitbot and mergebot both go through CI first and are linked to the github health checks.
Mergebot will not merge a PR if it fails the tests, commitbot only generates the PR (which goes through tests like any other commit).
1. We prefer to be able to say that everything is reviewed, but that in this particular case the review works automatically. Requiring a PR review is important to our security model (and therefore, our business). Skipping review, especially in a library that contains both these definitions and the code that leverages them would be too cavalier for us. 2. We want to give developers control over when these changes are merged. Many API changes are coordinated with other changes (e.g. in integrating services). We _could_ require backwards compatibility, but we find this too onerous in this layer of our stack.
re: test first; we have a very thorough process for qualifying releases:
- As much as we like continuous deployment, our (enterprise) customers aren't particularly interested in continuous changes to their experience and the $VALUE of a customer to us is far to high to take that kind of risk. - Practically speaking, we adopt the git-flow model and have separate deployment environments for integration (`develop` branch), QA (`release/` branch), staging (`tags/`) and then promote changes out to production, subject to suitable approvals/controls. I'm fully aware that this approach may sound cumbersome, but it's not in our line of business.
The most productive stacks I have worked with trimmed out stuff that wasn't needed for our particular organization. Every organization has common patterns they are used to and like. But, off-the-shelf stacks have to cater to a wide variety of organizations and organization preferences. Get a tunable stack and trim trim trim, and refactor duplication. Learn from each generation of product releases and make your API's better for the next.
Make special behavior as-needed plug-ins, not part of the main stack. Don't be a feature horder in the main stack, but keep a library of handy add-ons that are known to fit your stack when needed.
As an example of flexible factoring, you shouldn't normally have to specify the title and max length of an input field more than once (D.R.Y. Principle). If you have to keep doing it twice, find out why and factor it out. However, keep in mind that sometimes you do need to override the default title for specific needs such that you shouldn't remove the potential for task-specific customization in your effort to refactor. Use shared defaults for a field, but don't force shared defaults.
You should spend most of your coding time plugging in parameters to well-tuned API's, not copy and pasting repetitious code. Most of your actual coding should be for behavior that is unique to a project. Think twice, type once, instead of the other way around.
nevertheless, auto approving and merging pull requests and also auto bumping dependecies with only a bot as reviewer does sound like a very bad idea in general. how does one make sure the bump required for service A doesn't break service B which also relies that dependency? admitted, testing someone else's code is not the most exciting part of being a developer, but it's a necessary evil isn't it?
to be honest, this workflow sounds like hotfix hell to me when you'd combine it with continuous deployment...
Ok, but what about _all these small tasks over there_? They may also be small, but you have to think about them, work on them. Every second you are working on them you are not adding any value. Your brain gets tired, it makes everything feel so difficult to do, increasing friction on all tasks.
One thing that can't be overlooked: deploying changes to bots should also be seamless and automated as far as humanly possible. Otherwise they too will fall behind.
The library that interfaces with the bots is the same thing, once it's updated ALL of the builds will automatically use that latest version.
However it looks like you could build this out of combining bulldozer & another bot from palantir, if you really want to go no code.