Please confirm if that is all what we want from that pull request
yvanzo
O’Moin
alastairp
hi aerozol, sounds like a trip to K road is on the cards :)
aerozol: guidelines for CB sounds good - I guess the key here is that if we add them then we'll have to make sure that we apply them as well. In case you didn't know, we also have the MeB CoC which this should tie into directy
aerozol: re: CB design, I think that this might be a bit of a collaboration. I have the feeling that we should improve stuff, but I'm not entirely sure how or where we should start. Perhaps we could focus first on how to give a good indication if this is a MB or BB entity (I'll open a ticket for you)
now that CB is being used, I also wanted to sit down and have a mid-term planning session - e.g. where do we see it in 3-5 years, this could tie in well with a redesign. Maybe we could reserve some time during the summit to make a start on this
Freso
Just a heads up that I didn’t have surgery today. The surgeon just wanted to poke me a bit and ask some questions and I was in and out in less than 10 minutes. Sure glad I got up at 6 in the morning for this. :\ Anyway. Surgery in August/September, so I will be around tomorrow/Friday as usual.
aerozol: As alastairp mentioned, the MetaBrainz CoC applies to all MeB projects. Project specific guidelines are currently only really in place for MB. What do you feel isn’t covered by the MeB one for CB?
alastairp
Pratha-Fish: I'm here, what's your question? you can also directly ask it if you want - maybe someone else who is also online may be able to answer it without needing to wait fo rme
[musicbrainz-android] 14akshaaatt merged pull request #127 (03brainzplayer…bp-implementation): Service completion, Service callbacks and notification stuff created, basic service setup complete https://github.com/metabrainz/musicbrainz-andro...
[critiquebrainz] 14alastair merged pull request #438 (03master…sampledb-missing-entities): Always return dummy data in debug mode if it's not in MusicBrainz https://github.com/metabrainz/critiquebrainz/pu...
The mapping API seems to just return nothing when it can't find anything.
So When querying MBIDs in batches, we don't really know which output corresponds to which query
alastairp
ah!
Pratha-Fish
I tried solving it by joining the data received by the API to the original data on the basis of recording-MBID or recording_name. But I think it kinda defeats the purpose of the test, since when the data received by the API is different, it won't correspond to any particular row.
But now that I think of it, joining on the basis of recording_name could work
alastairp
yes, we discovered this when we first built it, and added a feature for this
note the two fields: recording_name (from musicbrainz) and recording_arg (the exact item that you passed in)
so you can compare the _arg fields from the result with your input to find the mappings
Pratha-Fish
Oh nicee
alastairp
you're right, because you have just looked up the recording.name field in the musicbrainz database, I suspect that in almost every situation you would be able to match against recording_name too, but we added the _arg fields exactly for this
Pratha-Fish
I noticed the arg fields, but didn't really understand what they meant
ouch, it seems we lack an alert for this one, I'll fix it
cert fixed
yuzie has quit
alastairp: I added checks for this
alastairp
👍
skelly37
zas: Could you start the tests for my gsoc PR? They seem to got stuck and cannot start automatically. Maybe because I've started a 2nd PR that modifies workflows too.
zas
skelly37: I can't, they are expected to run... not sure what's happening. You can push an empty commit to re-trigger them.
Also paging Dr. aerozol : Do you reckon this use of the website logos (under the entity name) is OK?
alastairp
Harry Potter and the Midnight Jazz bass player
monkey
The goal is to ensure users know which website they're reviewing/viewing an entity for
ansh
The mockups look great!
yuzie joined the channel
yuzie has quit
ZaphodBeeblebrox
Freso: :/ i hate when that happens
you get all ready for whatever and it's like , "nah, this was just a thing, the real thing is in months!"
aaaaaa
alastairp: thanks!
ZaphodBeeblebrox is now known as CatQuest
Lotheric__ has quit
Lotheric joined the channel
monkey
ansh: I have one more mockup of something alastairp and I discussed in person. Three things to note here: 1. the entity type selection navbar and 2. the BB brown color used for the overlays (we might want to make the MB ones purple instead of the current blue?) 3. the use of "Literary work" to separate from "Work" above. https://usercontent.irccloud-cdn.com/file/VZdbA...
That last point is something we thought about, not sure if it's necessary in this specific case considering each website has its own row of options
ansh
Using "Literary work" is quite good.
In the mockup, what does the 'See all' do ?
I mean why do we need it, We can always show all the entity tabs ?
Arsen_ is now known as Arsen
alastairp
ansh: it's the same as the current 'All' tab
it shows reviews of all types
ansh
Ohh. Maybe we can style it a bit later
monkey
Yes, I think it can be improved !
Just threw that together so we don't lose ideas
ansh
Yes, overall it looks really good :)
reosarevok
Heh. I didn't consider MBS-12476 until someone complained about it today, but it does seem like a proper annoyance
lucifer: hey, if you're around some time I'd like to have a chat about your debugging for these frontend tests. I saw your initial PR, this is weird!
lucifer
alastairp: hi!
alastairp
so running as the github runner uid, it fails (monkey and I were looking into similar stuff and it seems it might be related to node permission errors?)
but running as root succeeds?
lucifer
yes indeed
monkey
What changed? and when?
alastairp
did you manage to get some debug which showed the owner/permission of files? (I see you force pushed over the testing changes)
i sshed into the runner and ran `npm -v`. it failed so i started looking into the user permissions. the uid was set to 1001 which is the uid of the runner but the user was set to unknown so i tried to remove the user mapping and it worked.
right, my suspicion is github updated their runner around that point, so things broke down.
alastairp
the username was unknown? yeah, that happens if there's no entry in /etc/passwd for that uid
lucifer
yes
username unkown was inside the node container.
alastairp
how did npm -v fail?
lucifer
it printed nothing
alastairp
I wonder if it was something like a part of node requires looking up a username from a uid
e.g. imagine if github changed the default uid from 1000 to 1001
lucifer
no npm command worked fwiw. npm was unable to print help as well.
the only thing I'm worried about with your change is that if some things in the frontend tests write files, then on linux they will end up being owned by root during development
monkey
Magic.
lucifer
initially, i had planned to run as root only in CI but i thought it might lead to issues with tests failing locally working in CI or vice versa.
alastairp
yeah, right
if this fixes the issue for now, that's fine. but given our future plans to run everything as non-root we might have to re-think this again
lucifer
i am not sure if it is root though. it would be the default user of image if we don't specify --user no ?
I added the sql script and the tests for endpoint, and modified a bit based on lucifer's last review. Please feel free to look at it when you have time.