1 Apr 2024 |
Samantaz Fox | A problem I often encounter myself is "Oh, I'm fixing this, and I see this nearby code that can be fixed in a similar way, so let put the two together". And repeat that until ending with a PR that rewrites a lot of code. | 14:03:30 |
ibicha | I'm loving this conversation btw!
On the topic of big PRs, I frequently ask folks to split things up into smaller changes, because it drastically increases the time to approval, simplifies reviews, and increases overall attention of the reviewer. I agree with MOST of what's being said here https://google.github.io/eng-practices/review/developer/small-cls.html | 14:07:16 |
Samantaz Fox | Interesting, thanks for the link :) | 14:09:18 |
TheFrenchGhosty | Samantaz Fox: If you did read everything, I hope you didn't take the wrong way what I said. As I said I personally think it's going FAR too slow compared to what it could be, but I understand why you want to do it this way (even if I wouldn't do it this way) | 14:12:37 |
iv-bot | <dostoyevsky2> It's probably been already discussed but I don't understand how yesterday's fix works: https://github.com/iv-org/invidious/pull/4552/files | 14:14:21 |
TheFrenchGhosty | From what I understand it's using a value that the YouTube backend expect (a value that was found by reverse engineering the official clients I'm guessing) | 14:16:09 |
Samantaz Fox | TheFrenchGhosty I don't 100% agree with everything you said (e.x instances breaking). But other than that, I'm not perfect, and I'm open to positive criticism. | 14:17:48 |
Samantaz Fox | dostoyevsky2: in essence, when requesting data from Youtube, we're passing extra parameters. We don't know what these parameters do exactly (we don't have names to put on those values) but by observing what the official client does, we can deduce that this or that works/doesn't. | 14:19:33 |
TheFrenchGhosty | To be clear I didn't say "if instance break it's not a problem", what I meant to say was "if we can accelerate the speed at which things are going at the expense of instance stability/reliability, then so be it" | 14:19:59 |
Samantaz Fox | After that, it's a game of trying and guessing, until we find the right one. Maybe it's a debug parameter left here by Google employees? Who know. But all in all, that small number is currently the password to bypass other complicated client checks. | 14:20:28 |
TheFrenchGhosty | In reply to @samantazfox:pussthecat.org After that, it's a game of trying and guessing, until we find the right one. Maybe it's a debug parameter left here by Google employees? Who know. But all in all, that small number is currently the password to bypass other complicated client checks. I'm guessing they'll change this code/fix this loophole soon, they don't want another Vanced | 14:21:28 |
TheFrenchGhosty | It's temporary | 14:21:38 |
Samantaz Fox | TheFrenchGhosty It's not acceptable either, imho. If one day we move to stable releases, each release should be stable, as the name implies. Now that's kinda the same way with the merge batches I do. If you take the latest commit on master, it should always be stable. | 14:22:32 |
Samantaz Fox | Yes, this is temporary, as always. Last time was (checks notes) on Aug 6, 2023 | 14:23:26 |
TheFrenchGhosty | In reply to @samantazfox:pussthecat.org TheFrenchGhosty It's not acceptable either, imho. If one day we move to stable releases, each release should be stable, as the name implies. Now that's kinda the same way with the merge batches I do. If you take the latest commit on master, it should always be stable. Yeah as I said I understand why you want things to be this way, even if personally I wouldn't do it this way | 14:23:27 |
iv-bot | <dostoyevsky2> how is https://invidious.nerdvpn.de able to proxy the googlevideo.com link when you watch a video, is that an option one can select when installing invidious? | 14:33:01 |