[aur-dev] [PATCH 1/2] Add capability to pin comments above others

[Mark Weiman]

On Sat, 2015-11-28 at 13:06 +0100, Lukas Fleischer wrote:
> First of all, thanks for working on this! I planned on implementing
> this
> for a while but never got around to doing it.
> 
> The main difference of this series to what I had in mind is that you
> allow for pinning multiple comments whereas I planned to have a
> single
> pinned comment (per package base) only. Can you think of a use case
> where having several pinned comments is desirable? Can't the
> maintainer
> simply edit the pinned comment and add new information? Would we ever
> want to pin a comment from a user other than the current maintainer?
> 
> More comments on the implementation below.
> 

No problem!

I was thinking that rather than having to retype the same information
(or copy and paste), we could just pin a comment from anyone and if
multiple comments have good information, we can pin all of them. Only 5
get displayed unless you go to the "comments=all" page.

> 
> I did not have a very detailed look at the second patch in this
> series
> but is this really needed? Can't we simply retrieve all comments by
> calling pkgbase_comments() and order them such that pinned comments
> are
> displayed first?
> 

To begin, the only reason I made two patches is because my Postfix
server would complain when the patch got too long (lost connection
after DATA error) while using git-send-email, so I made two smaller
patches to get around that issue. Nothing I did could solve that
problem.

The idea I was going for was rather than removing the comment order
below (preserving the readability of a conversation), just make a copy
of it above. While doing that, make a new header above that says
"Pinned Comments" above the pinned comments, then do the normal "Latest
Comments" section. If you think it should be a single comment from the
maintainer that is displaced, I can make that change.

> 
> After having read the function name and the comment, I would have
> expected this function to print a pinned comment but that does not
> seem
> to be what this function does... Anyway, what is the difference
> between
> this function and pkgbase_pin_comment() below? Can't we somehow merge
> those functions?
> 

You know what, I cannot tell what I was thinking about when I made this
and it appears I never actually use this function. I removed it on my
own local instance of aurweb and nothing appears to be broken as a
result. The next patch revision will have this removed completely.

> 
> Isn't this equivalent to
> 
>     function can_pin_comment_array($comment) {
>         return can_pin_comment($comment['ID']);
>     }
> 
> ...? I also think that the function name is slightly misleading. I
> would
> have expected that can_pin_comment_array() accepts an array of
> comment
> IDs and returns an array of boolean values. Do we need that function
> at
> all? Can't we simply call can_pin_comment($comment['ID']) instead?
> 

Wow, I cannot believe that I didn't think of that. Whoops.

The reason I did this is it follows how it was done for editing and
deleting comments.  Even the comments are similar wording (all I really
did was a copy and paste from the others). I initially thought about
doing just the can_pin_comment() function, but I didn't want to differ
too much from editing and deleting.

I will for sure simplify that function, but if you think it's better to
delete it altogether, I'll get to work on making it work. Perhaps also
a rework the other two "array" functions (can_edit_comment_array() and
can_delete_comment_array()) should be done in the future if the
can_pin_comment_array() function is removed or the comment is changed.

Mark Weiman