On Fri, 11 Dec 2015 at 21:46:58, Mark Weiman wrote: > [...] > > Seems like pkgbase_pin_comment() and pkgbase_unpin_comment() are > > almost > > identical. Can we share most of the code? > > > > I thought about doing that, but I thought the names of the functions > were important for code readability. Perhaps adding an argument to > pkgbase_pin_comment() that does it and just have > pkgbase_unpin_comment() call the other function to change that > argument? Also the error messages have a slight difference in text that > also pushed me to making two separate functions. >
You might want to have a look at pkgbase_adopt() and pkgbase_vote() which do something similar. > [...] > > Why is this needed? Could you elaborate please? > > > > The reason I had to do this is the variable $base_id when opening the > file for a second time (to display all comments) was already set and > caused it to then be set to an empty string, so I added a check to see > if the variable was already set so if it is, it can avoid trying to set > it again removing the problem. I do not really understand why this > happens, but it does on my computer. > > It seriously took me a couple of hours of head scratching to find that > problem and fix it because it really shouldn't have been an issue. > > I also noticed that $base_id is set in both pkgbase_display_details() > and pkg_display_details(), should I just remove that assignment > altogether in pkg_comments.php? > [...] Yes, if removing this code doesn't break anything, it is definitely something we want to do. Maybe do it in a preparatory patch since it is quite unrelated to what this patch does. Thanks!
