[TSC-92] Breaking change not reverted when it should have been Created: 19/Apr/18 Updated: 30/Apr/19 Due: 05/Jul/18 Resolved: 28/Jun/18 |
|
| Status: | Closed |
| Project: | tsc |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Grievance | Priority: | Medium |
| Reporter: | Luis Gomez | Assignee: | Unassigned |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| ODL Project: | mdsal | ||||||||
| ODL Release: | |||||||||
| ODL Gerrit Patch: | https://git.opendaylight.org/gerrit/#/c/70769 | ||||||||
| Description |
|
When a valid upstream patch breaks downstream, the right thing to do is to file a weather report including explanation why the change is required and some pointers on how to fix the potential failures. This gives a chance for downstream projects to evaluate and accept the change as well as to prepare the required patches to minimize the impact. In this particular case the breakage was unintentional or unexpected so no weather was fired. However instead of reverting the patch, we went ahead with it because it turned out to be an "elaborated" revert and we wanted to unblock projects as soon as possible. While the "elaborated" revert argument is valid in exceptional situations like this one, this should not be by any means the way upstream changes get pushed in downstream and therefore this grievance is filed to make sure this practice is avoided in future. |
| Comments |
| Comment by Faseela K [ 20/Apr/18 ] |
|
The revert patch is on -2, and the ofp patch has been restored to adapt to the change. However there is a -1 from PTL on the patch. And now users are in deadlock.. Was there any post-TSC discussion on what is the way forward? rovarga, Avishnoi, dfarrell07, abhijit2511, jluhrsen ? |
| Comment by Michael Vorburger [ 20/Apr/18 ] |
|
How is the https://git.opendaylight.org/gerrit/#/c/71120/ an "elaborated" revert and an exceptional situation? It's Patch Set 1 passed the javadoc, verify and distribution checks, and got a Verified+1. It's Patch Set 2 just changed the commit message, and failed Distribution; but that was just our usual best friend odl-integration-all SingleFeatureTest Build timed out (after 120 minutes) - which we all know is unrelated to this revert. Personally I do not understand why we don't just revert that, and let the (heated) discussion on the list conclude something, then see patches for all affected projects which all PTLs of all those affected projects are OK with, then run a new "global build" (somehow, I don't know how) with that mdsal change re-reverted TOGETHER with all the changes dealing with the impacts in all the projects, run CSIT over that 7 times, and then merge all those changes together. And if that c/71120 is not the correct way to revert whatever caused all this confusion, then should it not simply be the responsability of the original author of said change to help reverting it? |
| Comment by Anil Vishnoi [ 22/Apr/18 ] |
|
I removed my -1 from the OFP patches. It's community decision, if they are okay with these changes. If community is fine with these changes and move forward, please feel free to merge the OFP patches. |
| Comment by Ariel Adam [ 16/May/18 ] |
|
Louis, who is the assignee of this grievance? |
| Comment by Ariel Adam [ 16/May/18 ] |
|
I take it back. I guess there is no meaning to a specific owner but rather taking it up with the TSC. I will change the dashboard to show the reporter instead of the assignee. |