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