[aur-dev] [PATCH v2] Implement capability to pin comments above others

[Mark Weiman]

On Sat, 2015-12-05 at 21:01 +0100, Johannes Löthberg wrote:
> 
> For starters, you've used spaces instead of tabs everywhere for
> indentation, so
> you're going to have to fix that, and won't comment on every instance
> of it, and it
> seems you forgot to add the details of the new SQL schema to schema/
> and upgrading/
> 

Just for clarification, I just modify the aur-schema.sql file to
include the new columns and for the upgrading file, do I put the query
for the change in the 4.2.0.txt or in it's own file?

> <snip>
> 
> 
> $show_only_pinned is misleading since this function doesn't show
> anything at all.
> Something like $only_pinned would be a more accurate name.
> 

I agree, it will be changed in v3.

> <snip>
> 
> <snip>
> 
> Both here and below you should check that the user has the right
> credentials,
> otherwise anyone that's logged in could send a POST request to pin
> any comment.
> 

I'm pretty sure I did the check (if (can_pin_comment($comment_id))). It
's pretty much based on the pkgbase_delete_comment() function, so if I
did indeed miss that here, that function should be checked as well.

> Should probably set a max number of pinnable comments as well.
> 

Seems reasonable, I'll make that change.

> <snip>
> 
> 
> The indentation is really all over the place here.
> 

Will be fixed in v3.

> <snip>
> 
> "Unin comment" and "Pin comment" instead of "Unpin comment"
> 

WHOOPS! I noticed that and I thought I changed it, but I guess it
slipped through.

Mark Weiman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: This is a digitally signed message part
URL: <https://lists.archlinux.org/pipermail/aur-dev/attachments/20151205/afab17ad/attachment.asc>