Fix public favorites #390

Open
lamp wants to merge 1 commit from lamp/akkoma-fe:public-favorites into develop
First-time contributor

closes #389

closes #389
Member

Thanks for porting this! I haven’t looked closely at this yet, but i think the attribution needs to be clearer. Both because attibuting original authors is the nice thing to do and because not doing so can become legally problematic.

iiuc the first commit ports over the two Pleroma PRs linked in the issue and those original commit hashes are mentioned in the commit messages. However, (a) there’s no information about the original author, (b) those commits don’t exists in akkoma-fe and thus can’t be easily looked up and (c) afaiu you’re not the original author of those patches.

If the original commits applied cleanly or only with minor changes. ideally they’d ideally retain the original primary author, possibly with an added Cherry-picked-from: line. (e.g. by exporting from pleroma-fe with git format-patch and then applying to akkoma-fe with git am, or just using git commit --amend --author="...")
If significant changes were needed, it’s probably fine to keep yourself as the primary author, but you should at least add something like this to the commit message:

Based on the below but with many adjustments
necessary to adapt it to akkoma-fe:

Cherry-picked-from: https://git.pleroma.social/pleroma/pleroma-fe/-/commit/6f452d672fe740035cf1d29d03bcda0d39438753
Cherry-picked-from: https://git.pleroma.social/pleroma/pleroma-fe/-/commit/1ceffb4e713b4b20d70121fba92d2b50f2d3cadf

Co-authored-by: marcin mikołajczak <git@mkljczk.pl>
Thanks for porting this! I haven’t looked closely at this yet, but i think the attribution needs to be clearer. Both because attibuting original authors is the nice thing to do and because not doing so can become legally problematic. iiuc the first commit ports over the two Pleroma PRs linked in the issue and those original commit hashes are mentioned in the commit messages. However, (a) there’s no information about the original author, (b) those commits don’t exists in akkoma-fe and thus can’t be easily looked up and (c) afaiu you’re not the original author of those patches. If the original commits applied cleanly or only with minor changes. ideally they’d ideally retain the original primary author, possibly with an added `Cherry-picked-from:` line. *(e.g. by exporting from pleroma-fe with `git format-patch` and then applying to akkoma-fe with `git am`, or just using `git commit --amend --author="..."`)* If significant changes were needed, it’s probably fine to keep yourself as the primary author, but you should at least add something like this to the commit message: ``` Based on the below but with many adjustments necessary to adapt it to akkoma-fe: Cherry-picked-from: https://git.pleroma.social/pleroma/pleroma-fe/-/commit/6f452d672fe740035cf1d29d03bcda0d39438753 Cherry-picked-from: https://git.pleroma.social/pleroma/pleroma-fe/-/commit/1ceffb4e713b4b20d70121fba92d2b50f2d3cadf Co-authored-by: marcin mikołajczak <git@mkljczk.pl> ```
lamp force-pushed public-favorites from 893ce69a1b to 4d91a7b2c3 2024-05-07 00:55:46 +00:00 Compare
Author
First-time contributor

ok i

ok i
Oneric reviewed 2024-05-12 23:42:47 +00:00
Oneric left a comment
Member

The basics seem good to me except for one change to potentially clean things up a bit

The basics seem good to me except for one change to potentially clean things up a bit
@ -117,2 +120,2 @@
// only we can see our own favourites
if (this.isUs) timelineTabMap['favorites'] = 'favorites'
if (this.favoritesTabVisible) timelineTabMap['favorites'] = 'favorites'
Member

provided it works well with user_profile.vue (not sure tbh; frontend stuff isn’t my strong suit), it would imho be cleaner to properly distinguish one’s own favourites from publicly accessible favourites here and get rid off the userId checks and mangling in the other parts, i.e.:

-      // only we can see our own favourites
       if (this.isUs) timelineTabMap['favorites'] = 'favorites'
+      else if(this.favoritesTabVisible) timelineTabMap['favorites'] = 'publicFavorites'
provided it works well with `user_profile.vue` *(not sure tbh; frontend stuff isn’t my strong suit)*, it would imho be cleaner to properly distinguish one’s own favourites from publicly accessible favourites here and get rid off the `userId` checks and mangling in the other parts, i.e.: ```diff - // only we can see our own favourites if (this.isUs) timelineTabMap['favorites'] = 'favorites' + else if(this.favoritesTabVisible) timelineTabMap['favorites'] = 'publicFavorites' ```
Member

oh also, can you add: Fixes: https://akkoma.dev/AkkomaGang/akkoma-fe/issues/389 as the last line of the commit message? This way the issue will be closed automatically (just mentioning it in the PR comment doesn't actually to work ime)

oh also, can you add: `Fixes: https://akkoma.dev/AkkomaGang/akkoma-fe/issues/389` as the last line of the commit message? This way the issue will be closed automatically (just mentioning it in the PR comment doesn't actually to work ime)
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u public-favorites:lamp-public-favorites
git checkout lamp-public-favorites
Sign in to join this conversation.
No description provided.