... | ... | @@ -72,12 +72,11 @@ There were some difficulties it would be nice to avoid in future, related to the |
|
|
* Rebasing such a PR when there's a conflict is painful and costs money and requires retesting and re-reviewing, even when it has smaller commits within a PR.
|
|
|
* Stuff can go missing.
|
|
|
* There's an incentive with big PRs to "get this merged before it goes stale", which is not an ideal.
|
|
|
2. (Theoretical, but has happened elsewhere.) Possibly another PR that gets merged requires a change to the code, but it gets rebased verbatim, and doesn't get retested and then fails later.
|
|
|
* CiviCRM developers and end users following the TZ Issue and PR were either unable too or had significant barriers to test and review the PR. It was only when CiviCRM 5.47 was released when they could download and try it out for themselves. This significantly reduced the number of people able to be involved in reviewing and providing feedback.
|
|
|
* Changing the date/time for a system to test in pre-DST and post-DST environment is difficult for many and in cases where CiviCRM is on a hosted service, impossible.
|
|
|
2. (Theoretical, but has happened elsewhere.) Possibly another PR that gets merged requires a change to the code, but it gets rebased verbatim, and doesn't get retested and then fails later.
|
|
|
3. (Theoretical/Opinion) It is offputting and difficult to do code review for such PRs. Perhaps fewer people than normal looked at the code.
|
|
|
|
|
|
It was mentioned at one point in the review to break it up, but it seemed difficult to do at the time. Perhaps if being refactored it can be planned in a way this can be achieved.
|
|
|
4. It was mentioned at one point in the review to break it up, but it seemed difficult to do at the time. Perhaps if being refactored it can be planned in a way this can be achieved.
|
|
|
|
|
|
<a name="affected"></a>
|
|
|
## Appendix: Affected components
|
... | ... | |