[aur-dev] [PATCH v2] Implement capability to pin comments above others
Mark Weiman
mark.weiman at markzz.com
Sun Dec 6 03:51:05 UTC 2015
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>
More information about the aur-dev
mailing list