-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add automated pre-merge testing procedures to match OpenROAD's ambitions #23
Comments
I agree 100% with everything you have said. Thanks |
@tajayi It warms my heart to hear this :-) It's not easy to change the culture and infrastructure in an organization, but I'm convinced that OpenROAD stands no chance of reaching it's goals without a great community culture and quite an advanced, highly automated and robust automerge infrastructure To implement this requires real money and is a lot like work, but it is absolutely imperative to have this in place to make effective use of contributes time. If this is not in place, nobody will dare touch code that's brittle and in dire need of being fixed. TIP! Read up on git subtree when thinking about how to create this infrastructure. What you can do with git subtree is that you can:
The advantage of this approach are many:
|
I've discovered that auto-merge must be a fairly common practice at github. There exists automation infrastructure as GitHub Apps. The implementations I've found work as follows:
At this point, a pull request will automerge if configurable policies are met, typically automated testing passed and sufficient approval Two implementations that I Googled, which appear very similar: https://github.com/apps/probot-auto-merge |
OpenROAD has quite ambitious goals, so the automated pre-merge testing procedures need to be quite ambitious.
Once OpenROAD community gets going, there's easily going to be 100s of pending pull requests in flight across the projects and dozens of developers who depend on the stability of the master branch to work effectively.
It's trivial to manage a pending pull request: don't merge until it's completely ready, whereas it is an absolute nightmare, not to talk about catastrophic productivity loss, to manage unfinished work in the master branch that's interfering with development.
Another challenge for OpenROAD is that the automated tests will not scale easily. Each module should have a substantial amount of testing unique to that module, say up to 1 hour per pull request, but beyond that there needs to be quite substantial integration tests that tests the full flow. This full integration is probably going to have to be on the order of 24 hours.
Fortunately, these days this is a well studied problem from the world of DevOps, so solutions exist and are well known.
Rough outline of an example of how this can be achieved:
Closing words:
I've had this sort of system in place for large complicated projects before and while it takes getting used to, it is absolutely fantastic once you get it going.
There are the technical benefits, but more importantly it greatly improves coordination and communication for pending pull requests and it creates a working environment where developers feel safe: they can't do anything wrong, the system will catch it.
Additionally, it's a great boost to overall productivity. Writing the code is a tiny part of a developers job. The majority of the work is in re-testing, coordination and debugging with the rest of the system.
Merging directly to master gives developers immediate gratification. Immediate gratification breeds addiction and terrible habbits. The prime example is that a developer merges something with master that someone else has to fix or complete: this abrasive behavior improves the productivity of the developer who merges with master branch at the cost of the productivity of everyone else.
The above is essentially a delayed reward system: only when everything is complete, does the developer get the reward of merging.
This approach maximizes the productivity of the team as a whole, rather than focusing on individual developers.
The text was updated successfully, but these errors were encountered: