[PATCH 0 of 7] save multiple comments at once

[Jan Heylen]

On Fri, Apr 24, 2015 at 7:18 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 04/14/2015 06:03 PM, Jan Heylen wrote:
>>
>> Hi,
>>
>> I've been working on a change that a user would allow to open multiple
>> comments on a commit, and 'post' (save) them all at once to the
>> kallithea database.
>
>
> I did it! I looked at it, tried it, and did some quick review of it! ;-)
>
> It works and I can see how it can be useful. But it is not as the UI or the
> implementation just seems like an elegant solution where everything just
> fits together. This PR is not to blame for that. It is more that the
> existing system isn't geared for this change. I would like to move towards a
> model where we can say that this feature doesn't add any technical debt -
> I'm not sure we can say that yet.

Can you be very specific on this? What part is not geared, where we
add technical debt with this? Holding back patches because the rest of
the code isn't perfect sounds like a bad plan to me, but hey, if that
is your plan, go ahead, but do be very clear, it's either possible to
add features on the existing code base, or it is not, and then you
should freeze development, make it clear to people in a non
interpret-able manner and start cleanup/refactoring, as yes, I see too
that the backend code could use re-factoring or cleanup. I even
believe the frontend requires a complete start-over (and create e.g.
an angular application), but I'm just not that angular web developer
or database model expert, I just want to add a feature into the
existing code base that I believe is useful and required. I'll assume
for now you still want to consider taking in the patches (after
updates) but please be very clear if that is not the case.

Btw, I worked with some very good webdevs, people like that can do
quite some work in a week or so if you would start from scratch, it is
as always, if you know what you're dealing with....

>
>> This patchset implements this workflow, by changing the 'comment' button
>> on inline comments to a 'save' button and having a bottom 'commit all
>> comments'
>> button that will commit these draft comments as real comments.
>
>
> A general comment: I would expect "commit all comments" to also save drafts
> I haven't saved yet. (That would render the inline save buttons kind of
> pointless ... which I think is fine if we want people to think of it as
> making all comments at once.)

Yes, that could be a good idea, remove all the buttons from the inline
comments, do "automatic" preview when leaving an edit box, If possible
I'll post an update for this idea


>
> Also, it should not be possible to create more than one draft comment for
> each line.

Could be useful if your comments are not related to each other, but
I'll see to add a check for only one draft

>
> There is also an problem with deleting my own draft comments on closed PRs.

Didn't test that, I'll need to have a look.

>
>>
>> That way, it becomes possible to review a whole commit, and only submit
>> your
>> comments when you're finished reviewing the commit, and only one summary
>> email is sent.
>
>
> I think it becomes essential to have a way to navigate through the
> pending/draft comments. They must be linked into the next/previous comment
> chain as they are opened or cancelled. And there should probably also be a
> way to navigate all the pending comments, perhaps with a list next to the
> big save button.

At the moment, once saved (and that could be 'automatic' when leaving
the edit box), the next/previous chain is working.
The list could also be possible, but I don't see the real added value
on that, although a tool like Gerrit also has it I believe.

>
>>
>> As this changes the workflow quite a bit, a remaining question is if this
>> behaviour needs to be configurable or not.
>
>
> I don't think so. It just has to be so good that everybody wants it ;-)
>
> /Mads