Embed Notice
HTML Code
Corresponding Notice
- Embed this notice
narcolepsy and alcoholism :flag: (hj@shigusegubu.club)'s status on Tuesday, 28-Nov-2023 22:26:46 JST narcolepsy and alcoholism :flag:
@NEETzsche @lain @Moon >Give me real problems in terms of any of these things and, as I have shown, I will work with you on them. This is because I want to make things better.
Here:
- Having Listen activity with just URLs pointing to outside of network or nonsensical data is tethering on being spam (same as Likes on nonexistent statuses), it also can conflict with other software that might want to use Listen for counting listens on actual Audio documents.
- url field according to https://www.w3.org/TR/activitystreams-vocabulary/#dfn-url is meant for url pointing representation of object, so for a Listen activity it would mean url should point towards the representation of listen activity itself, not what has been listened to.
- if you want to link towards what is actually being listened to you should use "target" field instead, with description of Link object https://www.w3.org/TR/activitystreams-vocabulary/#dfn-link and add rel=['external', 'nofollow'] for arbitrary data to suggest servers not query it.
Additionally, for MR about including scrobbles in user profile:
- Profiles are typically cached and mostly static (people don't change their bios/avatars several times during day unless they are testing/actively adjusting things which are rare), scrobbles are transient and dynamic and can happen dozen times per day, so having them in user profiles will either lead to removing/reducing the cache or showing outdated scrobbles due to cache. We have /relationships endpoint for fetching non-cached data like following/blocked/etc statuses, so either we should extend that or keep it a separate endpoint as it is now. Ideally scrobbles should just be just part of the timeline and/or websocket stream (like deletes).
>Here's how to do a code review:
Can you funking stop patronizing and trying to teach people how to do things?
Here's how I do merge request reviews :bigband:
0. Check if it is a spam or outright malicious/troll MR like "remove entire codebase because it stinks" and if so, close it outright.
1. Look at the point of the MR. Do you agree with it? If not, state why. Wait for response.
1.1. Estimate maintenance and responsibility costs and decide if you're willing to take the risk as a maintainer. If not, have someone else to take care of it if possible.
1.2. Check whether MR affects any other usecases and if it needs to be made configurable and what default should be if so.
2. Look at the diffs. Are they good? If not, state why. If it's irredeemably bad, say so.
2.1. Decide if some of the changes you might want are too difficult/complex for author and too easy for yourself to quickly implement, or implement in future.
3. Test MR locally, if there are any serious issues and design flaws, mention them.
3.1. Estimate risk if there are minor flaws and take responsibility for fixing them.
4. Dogfood MR for some time and if everything is relatively stable and usable, and all the discussions/issues are resolved hit merge.
5. ???????
6. PROFIT!!!