ruaok: I was thinking that we could just go through each PR from bottom to top and decide if 1) ready to merge, 2) looking good and we should push its author to rebase and fix pending issues, 3) punt it until later
2020-12-11 34603, 2020
Mr_Monkey
I had a look through #1086 and I don't mind taking it over going forward. Another pair of eyes welcome though.
2020-12-11 34603, 2020
Mr_Monkey
I've also looked at #1048 (LB-571) which solves the issue at hand only for a handful of routes; is there a way to have a more generic solution?
Mr_Monkey: I'm happy to take over 1048, as far as I know, this is the correct way to do it, we just need to consistently write our routes like this
2020-12-11 34637, 2020
Mr_Monkey
OK.
2020-12-11 34600, 2020
Mr_Monkey
I tested the ones that have been corrected, and everything works well AFAICT
2020-12-11 34616, 2020
Mr_Monkey
(with and without the final `/`)
2020-12-11 34634, 2020
Mr_Monkey
Is `/` an allowed username character?
2020-12-11 34608, 2020
alastairp
yeah, I think we just got it wrong when we started
2020-12-11 34614, 2020
alastairp
good question about usernames, I don't know
2020-12-11 34602, 2020
alastairp
mm, we'll have to test API endpoints, especially POST ones. some HTTP clients don't act well if they get a redirect on a POST
2020-12-11 34624, 2020
adhawkins has quit
2020-12-11 34641, 2020
adhawkins joined the channel
2020-12-11 34635, 2020
D4RK-PH0_ has quit
2020-12-11 34616, 2020
D4RK-PH0ENiX joined the channel
2020-12-11 34637, 2020
D4RK-PH0_ joined the channel
2020-12-11 34637, 2020
Nyanko-sensei has quit
2020-12-11 34633, 2020
BrainzGit
[listenbrainz-server] MonkeyDo merged pull request #1202 (production…dependabot/npm_and_yarn/production/ini-1.3.7): [Security] Bump ini from 1.3.5 to 1.3.7 https://github.com/metabrainz/listenbrainz-server…
shivam-kapila: 2 comments on that PR, it'd be nice to have a more detailed description explaining what it's for and how it'll be used. Just looking at the PR I have no idea why we want this
2020-12-11 34624, 2020
alastairp
be careful about your commit messages too - "Start fresh... New schema" is very vague and doesn't tell me anything about what this feature is for
2020-12-11 34643, 2020
shivam-kapila
alastairp: will make it more descriptive
2020-12-11 34650, 2020
alastairp
#1048, as I mentioned to Mr_Monkey just before, I'll take this and finish testing and make any other changes
2020-12-11 34619, 2020
alastairp
#1086, Mr_Monkey says that he will take this on
2020-12-11 34647, 2020
alastairp
#1104, I don't know how to evaluate this
2020-12-11 34652, 2020
ruaok
#1104 has no tests and is stale. I'd say close for now.
https://github.com/metabrainz/listenbrainz-server…, I'm 98% sure that this isn't going to cause any problems. We already have integration tests to check that json in/out is working properly. It doesn't look like there were any major changes from a quick read of the release notes
2020-12-11 34640, 2020
ruaok
Mr_Monkey: ok
2020-12-11 34619, 2020
alastairp
#1188, I'll test this and merge, again I don't see any major reason why it might break any exsting code that we have
Mr_Monkey: alastairp : the edit_playlist endpoint is done, save for the collaborators part. I'll have to make another pass on things once that is implemented. but other stuff should all be working now.
