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.
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.)
Also, it should not be possible to create more than one draft comment
for each line.
There is also an problem with deleting my own draft comments on closed PRs.
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.
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
_______________________________________________
kallithea-general mailing list
[email protected]
http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
