Over the last few days we have analyzed the reasons why version 8 release was not a smooth process and the core reason we have identified is
More lines of code per change = more chance of breaking and bugs
Also more lines of code are harder to evaluate, test and has more potential damage. So we are proposing a change in the way contributions are made to ERPNext.
We expect that you send pull-requests that change / add upto 30 lines of code excluding tests, templates and config files in one go. If you are working on a large feature, please break down your feature into small parts and get your contributions merged in parts.
Edit: We will go soft on the 30-lines limit as long as the contribution has test coverage, and enough screenshots and documentation.
We will now accept incomplete features as long as they are hidden and the path to the next contribution is clear. This means that if you are doing a feature, please add tasks in your GitHub issue and send a PR for each task.
This is a change in our current policy where we did not accept incomplete features.
Also we will avoid doing big releases from now on. Weekly releases with fixes will be on Tuesdays and Thursdays.
Good idea I think, easier to review, catch problems and avoid regressions.
Letās see if always possible to break in about 30 lines, depending of features.
It will also help to merge PR quickly.
What about the merge frequency from develop branch into master ?
Because when sending PR to develop branch, it is easier to wait for getting back the master branch for production, than playing with git cherry-pick to deploy the PR.
Iāve seen other projects use a monthly āfeature releaseā cadence with good success. Maybe āhotfixā can be merged on Tuesday and Thursday, and ādevelopā is merged once a month, maybe the second or third Thursday of a month.
We will start implementing a new strategy for large pull requests:
We donāt want to have develop diverge a lot more and then put resources in testing it at the time of merge. The goal is to try a lean system (inspired by lean manufacturing). WIP is un-shipped code. Un-shipped code should be minimum.
If we are able to achieve that, we will get speed + cost + quality.
In this sequenceā¦there is another think I donāt know if is already defined / documented. There are lots of github issues in erpnext repo that is about frappe (labeled framework) and others create directly in frappe.
Should I create issues on erpnext only or should (like how I actually do) create issues in frappe and erpnext differently?
I really appreciate the problem you are trying to solve. It is very likely to work well for small fixes which generally donāt result in adding/changing too many lines of code. But I donāt think 8.x issues are due to those kind of defect fixes.
The points you have mentioned in Cascading Pull Requests Ā· frappe/erpnext Wiki Ā· GitHub make perfect sense. In agile development, it is fundamental to breakdown the work into small testable chunks. However I am not quite sure everything can be broken down into 30 lines limit. If imposed strictly, it is likely to have adverse effect on the remote contributions, where turnaround time for pull requests are going to be significant compared to local contributions. Though small change are easy to review, it might be difficult to manage with big features. The overhead of context switch and frequent changes are very likely to overburden reviewers and gatekeepers defeating the very purpose.
Looking for the significance of number 30, I came across http://swreflections.blogspot.in/2012/12/rule-of-30-when-is-method-class-or.html. An alternate approach based on that would be to enforce 30 line rule for functions, classes and files rather than for pull request. In addition to that, we should also mandate automated tests where applicable. Those items coupled with a prior feature design document should make the code/feature much better to manage.
Keep the feature hidden from the user until it is complete
This is better done with framework support. One approach is to add āDevā checkbox along with āBetaā, which makes the feature available only on ādevelopmentā mode. Perhaps there is more to do, but it is a good starting point.
We have to try different strategies. 30 is more a recommendation rather than a hard number. We donāt want to compromise either ā speed or quality. So we have to see what works best.
A quick update here. I think we will go soft on the 30-lines limit as long as there is enough documentation, tests and coding standards are adhered to.
@rmehta Hi, I think the way of closing pull requests while waiting for informations or corrections is a bad habit.
Indeed, it is impossible to re-open it because it is not closed by ourselves.
Then we need to ask for opening it back, if you have no time and we donāt check to ask again, PR can stay closed and will never be merged.
6 days Iām trying to re-open a PRā¦
The process needs to be smooth to encourage contributions.