<table><tr><td style="">rjvbb added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D4981" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>It adds a bit of complexity, but it looks worse than it is if you ask me. It's in fact not much more than a single additional argument; the complexity stems more from the number of places where that argument has to be added than from the changes themselves.</p>

<p>In turn that stems from the complexity of the whole underlying design. It's not the first time I set out to make what I thought would be a simple change was annoyed to find I needed to make way more changes in way more hard to find places than you'd expect. It was indeed my plan to test the whole idea only with git, but I couldn't see a way to put the spinbox where it is now, make it trigger a patch review update and access its information only in the git plugin. Besides, the feature should probably be implemented for the other VCS flavours supported by Phab, at least svn.</p>

<p>Maybe we could use the occasion to change the API to pass the arguments that have default values via a pointer to a class or struct which can hopefully be defined completely in the base class?</p>

<p>Or maybe this new parameter can be implemented as a member variable in <tt style="background: #ebebeb; font-size: 13px;">IBasicVersionControl</tt> rather than as a function argument? The class already has a setter for another property so it isn't purely abstract, and this would mean a lot less passing around of an additional variable. I'll have a look at that approach, see if it makes the patch less complex.</p></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4981" rel="noreferrer">https://phabricator.kde.org/D4981</a></div></div><br /><div><strong>To: </strong>rjvbb, KDevelop<br /><strong>Cc: </strong>apol, kdevelop-devel, KDevelop, Pilzschaf, akshaydeo, surgenight, arrowdodger<br /></div>