-
yvanzo
Probably not by adding 'data-score' even though it is not needed.
2019-12-12 34648, 2019
-
yvanzo
score can be accessed through row.original.score
2019-12-12 34640, 2019
-
rahul24 joined the channel
2019-12-12 34634, 2019
-
yvanzo
Spread syntax (...) can probably be used to add 'data-score' only when needed.
2019-12-12 34607, 2019
-
reosarevok
It's not added if it's null though? At least not currently, where we use the same code for instrument collections and those have no data-score
2019-12-12 34654, 2019
-
yvanzo
Oh great then :)
2019-12-12 34658, 2019
-
yvanzo
Oops, that was a question! No, it should not be added when it’s null.
2019-12-12 34628, 2019
-
reosarevok
No, it wasn't (quite) a question :D
2019-12-12 34618, 2019
-
reosarevok
For InstrumentList I'm doing return <Table columns={columns} data={instruments} />
2019-12-12 34640, 2019
-
reosarevok
But on InstrumentResults we have "result", with result.instrument and result.score
2019-12-12 34615, 2019
-
reosarevok
Not sure what'd be the best way to make both use the react-table definitions :)
2019-12-12 34641, 2019
-
reosarevok
I guess I could map the instruments from results into an instruments-only array, but what about the score?
2019-12-12 34642, 2019
-
yvanzo
results should be rows
2019-12-12 34627, 2019
-
yvanzo
accessor can be tuned to get instrument data
2019-12-12 34606, 2019
-
BrainzGit
2019-12-12 34632, 2019
-
CatQuest
reeeo
2019-12-12 34643, 2019
-
reosarevok
But then we have the case where say defineNameColumn would need to know to access either "name" or "instrument.name"
2019-12-12 34644, 2019
-
CatQuest makes the demanding catnoise
2019-12-12 34651, 2019
-
reosarevok
(and then it would be "area.name" or whatnot)
2019-12-12 34601, 2019
-
CatQuest
reeeeoow
2019-12-12 34630, 2019
-
yvanzo
reosarevok: right, adding a parameter to defineNameColumn should work.
2019-12-12 34615, 2019
-
reosarevok
So we should have a parameter like isSearch passed to all columns?
2019-12-12 34643, 2019
-
reosarevok
Wait, no, I'd have to pass the name of the results object
2019-12-12 34647, 2019
-
pristine__
ruaok: have merged #60 to all-changes-mapping, tests will now have this branch as base
2019-12-12 34608, 2019
-
reosarevok
Sigh, why haven't we just put the score as part of the instrument or whatnot to make our life easier yet :p
2019-12-12 34639, 2019
-
CatQuest
instrument?
2019-12-12 34603, 2019
-
CatQuest reads the scrollback with renewed interesst
2019-12-12 34636, 2019
-
yvanzo
reosarevok: not isSearch but more likely accessorPrefix, so we can use it for any table (search results, reports, collections, whatnot…)
2019-12-12 34653, 2019
-
rishikesh has quit
2019-12-12 34603, 2019
-
yvanzo
bitmap: I would have suggested lens for the above accessorPrefix, but it has been removed months ago. Do you have any other suggestion here?
2019-12-12 34604, 2019
-
c1e0_ joined the channel
2019-12-12 34609, 2019
-
c1e0 has quit
2019-12-12 34606, 2019
-
rahul24 has quit
2019-12-12 34632, 2019
-
travis-ci joined the channel
2019-12-12 34632, 2019
-
travis-ci
2019-12-12 34632, 2019
-
travis-ci has left the channel
2019-12-12 34603, 2019
-
yvanzo
reosarevok: stupid me, no need of additional dependency, just row[accessorPrefix].name
2019-12-12 34622, 2019
-
reosarevok
So, would you suggest doing something like accessorPrefix ? row[accessorPrefix].name : name?
2019-12-12 34639, 2019
-
travis-ci joined the channel
2019-12-12 34639, 2019
-
travis-ci
2019-12-12 34639, 2019
-
travis-ci has left the channel
2019-12-12 34638, 2019
-
bitmap
is it row.name in one case? I'd suggest defineNameColumn should accept a function which returns the name from the row
2019-12-12 34655, 2019
-
yvanzo
that is accessor :)
2019-12-12 34656, 2019
-
yvanzo
accessor can be either a string or a function in Column definition
2019-12-12 34656, 2019
-
bitmap
from what I understood one is row.original.name and the other is row.original.instrument.name
2019-12-12 34627, 2019
-
bitmap
one problem is that Flow won't like using a string indexer there at all but it should understand a function fine
2019-12-12 34613, 2019
-
bitmap
zas: should the postgres data dir on williams be moved to a home directory? right now it's in /var/lib/docker/volumes
2019-12-12 34645, 2019
-
yvanzo
2019-12-12 34656, 2019
-
yvanzo
2019-12-12 34629, 2019
-
bitmap
yeah, it works now 'cause CoreEntityT | CollectionT both have a 'name' property
2019-12-12 34636, 2019
-
reosarevok
Well, not just there, but everywhere really
2019-12-12 34643, 2019
-
reosarevok
Everywhere that we'll need for searches
2019-12-12 34659, 2019
-
bitmap
but a result object with {instrument, score} doesn't
2019-12-12 34623, 2019
-
reosarevok
Soooo, what's the suggestion here? :D
2019-12-12 34634, 2019
-
yvanzo
bitmap: not sure I expressed correctly my suggestion, it is to replace the value 'name' here with an 'accessor' argument passed to 'defineNameColumn'.
2019-12-12 34615, 2019
-
zas
bitmap: the main problem is Jenkins, built images aren't cleaned, perhaps we should just move jenkins to a VM or smt
2019-12-12 34618, 2019
-
reosarevok
I mean, I could just pass data={a map of instruments} and scores={[entityId: string]: number}...
2019-12-12 34643, 2019
-
bitmap
zas: at least mb doesn't use jenkins for tests anymore, though we do use it for building docker images
2019-12-12 34607, 2019
-
yvanzo
'accessor' argument can default to the string value ''name'' or be passed as either a string or a function
2019-12-12 34610, 2019
-
bitmap
don't think that creates any volumes I'll check the config to make sure
2019-12-12 34611, 2019
-
bitmap
mb could even start using circleci to build docker images, they have a thing to save the docker build cache in their cloud or whatever
2019-12-12 34624, 2019
-
yvanzo
2019-12-12 34635, 2019
-
bitmap
yvanzo: that makes sense to me, I'm just not sure Flow will like it defaulting to a string (but I haven't checked)
2019-12-12 34658, 2019
-
bitmap
and the Cell would need customizing too since original isn't the entity anymore
2019-12-12 34603, 2019
-
yvanzo
thus my initial suggestion to just pass an accessorPrefix (or entityPath) but that is probably not flow-friendly.
2019-12-12 34653, 2019
-
kkmonster joined the channel
2019-12-12 34635, 2019
-
bitmap
a function that takes row.original and returns the entity should work fine for both
2019-12-12 34624, 2019
-
bitmap
then the accessor would be row => getEntity(row.original).name
2019-12-12 34649, 2019
-
bitmap
it could default to lodash/identity so you don't have to specify it
2019-12-12 34606, 2019
-
reosarevok
So how would getEntity work?
2019-12-12 34618, 2019
-
BrainzGit
2019-12-12 34623, 2019
-
reosarevok
At first I thought it'd just look whether original.(any entity name) exists, but we have artist.area and whatnot so
2019-12-12 34615, 2019
-
bitmap
it'd have to be different for each search list
2019-12-12 34631, 2019
-
reosarevok
Oh, so you define the function and pass it
2019-12-12 34641, 2019
-
bitmap
yea
2019-12-12 34642, 2019
-
reosarevok
to Table
2019-12-12 34600, 2019
-
bitmap
dunno, I forget how it works already :)
2019-12-12 34615, 2019
-
Wassabi has quit
2019-12-12 34639, 2019
-
reosarevok
Tsk :p
2019-12-12 34616, 2019
-
bitmap
if you want you can update react-table to rc.9 in whatever pr you're working on too
2019-12-12 34608, 2019
-
reosarevok
Any useful changes from it?
2019-12-12 34631, 2019
-
bitmap
probably just bug fixes, we're on an older beta version rn
2019-12-12 34635, 2019
-
c1e0_ has quit
2019-12-12 34641, 2019
-
rahul24 joined the channel
2019-12-12 34654, 2019
-
rahul24 has quit
2019-12-12 34639, 2019
-
kkmonster has quit
2019-12-12 34608, 2019
-
sbvkrishna has quit
2019-12-12 34627, 2019
-
rahul24 joined the channel
2019-12-12 34654, 2019
-
bitmap
yvanzo: we should prob set DBD::Pg to 3.10.0 in the cpanfile too, the json dumps broke without it (it has some fixes for pg 10-12)
2019-12-12 34618, 2019
-
BrainzGit
2019-12-12 34619, 2019
-
BrainzBot
2019-12-12 34623, 2019
-
rahul24 has quit
2019-12-12 34605, 2019
-
Wassabi joined the channel
2019-12-12 34614, 2019
-
ruaok
iliekcomputers: we never really followed up on the settings goals/deadlines convo, did we? still want to do that?
2019-12-12 34607, 2019
-
Wassabi has quit
2019-12-12 34625, 2019
-
iliekcomputers
ruaok: hi! Yes! Maybe Sunday or after? I'm in SF this week and will probably be really jetlagged Saturday.
2019-12-12 34640, 2019
-
ruaok
gawd, I know that feeling!
2019-12-12 34612, 2019
-
ruaok
I'm off some for xmas market fattening up over the weekend, so that works out well. monday or maybe tuesday evening?
2019-12-12 34641, 2019
-
iliekcomputers
Either would work for me.
2019-12-12 34650, 2019
-
ruaok
and at some point I'd love for you to review the messybrainz-labs project.
2019-12-12 34607, 2019
-
ruaok
not that there is a PR, but there is a whole new project now.
2019-12-12 34615, 2019
-
iliekcomputers
Yeah, I've been meaning to get up to speed on that
2019-12-12 34621, 2019
-
iliekcomputers
I also had some questions
2019-12-12 34630, 2019
-
iliekcomputers
On whether it needs to be a whole new project
2019-12-12 34646, 2019
-
iliekcomputers
Or if it could probably stay in the messybrainz repository
2019-12-12 34650, 2019
-
ruaok
not sure I have an answer yet.
2019-12-12 34609, 2019
-
iliekcomputers
With the ListenBrainz and labs decomposition, we had to duplicate some work
2019-12-12 34611, 2019
-
ruaok
for starters I wanted a project that I could noodle about and test some things.
2019-12-12 34615, 2019
-
iliekcomputers
Like tests for starters
2019-12-12 34637, 2019
-
iliekcomputers
ruaok: I agree with you on doing it with a clean slate initially
2019-12-12 34642, 2019
-
ruaok
luckily the testing worked out and its a real viable project. (one that no longer uses 50GB of ram either!)
2019-12-12 34600, 2019
-
iliekcomputers
I was more wondering if its worth to try to move it over to the main repo
2019-12-12 34607, 2019
-
ruaok
but I'm happy to merge it into the msb repo -- it would remove duplication, you're right.
2019-12-12 34629, 2019
-
ruaok
MSB or LB "main repo"?
2019-12-12 34637, 2019
-
iliekcomputers
msb
2019-12-12 34644, 2019
-
ruaok
yep, agreed.
2019-12-12 34607, 2019
-
iliekcomputers
To be honest, if we were starting everything rn, I would have maybe argued for keeping EVERYTHING in a big repo
2019-12-12 34608, 2019
-
ruaok
then I will not implement things like manage.py, since those will magically appear later.
2019-12-12 34626, 2019
-
iliekcomputers
LB, AB, CB all of it.
2019-12-12 34626, 2019
-
ruaok
that is what google does and I see it to a degree.
2019-12-12 34656, 2019
-
ruaok
I'm certain I dislike the old BB way of having 5 or so repos.
2019-12-12 34603, 2019
-
iliekcomputers
It would make brainzutils redundant, and sharing code really easy
2019-12-12 34604, 2019
-
ruaok
confusing as hell.
2019-12-12 34613, 2019
-
ruaok
better for security too.
2019-12-12 34617, 2019
-
iliekcomputers
Indeed
2019-12-12 34622, 2019
-
iliekcomputers
Just one place to update
2019-12-12 34637, 2019
-
ruaok
well, right now that isn't fully possible is it?
2019-12-12 34642, 2019
-
ruaok
AB can't be py3 yet can it?
2019-12-12 34657, 2019
-
ruaok
which is scary in an of itself.
2019-12-12 34600, 2019
-
ruaok
and
2019-12-12 34625, 2019
-
iliekcomputers
Migrating AB should be a small task, it's the ML libraries behind it (essentia?) that's hard
2019-12-12 34630, 2019
-
ruaok
exactly.
2019-12-12 34649, 2019
-
ruaok
how exactly is that invoked? if that is via the command line, then fine.
2019-12-12 34600, 2019
-
iliekcomputers
I am not sure.
2019-12-12 34604, 2019
-
ruaok
if we're liking to python bindings that are 2.x... ugh.
2019-12-12 34621, 2019
-
ruaok
I could covert to calling an external binary and move the web base to 3
2019-12-12 34638, 2019
-
ruaok
convert
2019-12-12 34626, 2019
-
iliekcomputers
Hmm, need to get some alastairp time on this, I guess.
2019-12-12 34643, 2019
-
ruaok
aww yiss. test now all pass.
2019-12-12 34645, 2019
-
iliekcomputers
To discuss the best way forward
2019-12-12 34659, 2019
-
ruaok
these tests have been saving my ass in making sure things are working right.
2019-12-12 34631, 2019
-
ruaok
might be goo to do that when you are in BCN.
2019-12-12 34656, 2019
-
iliekcomputers
Yes!
2019-12-12 34601, 2019
-
iliekcomputers
Also
2019-12-12 34607, 2019
-
iliekcomputers
This maybe a bad idea
2019-12-12 34609, 2019
-
iliekcomputers
But
2019-12-12 34628, 2019
-
iliekcomputers
Do you think we should evaluate merging labs into LB too
2019-12-12 34656, 2019
-
CatQuest
iliekcomputers: 🍺
2019-12-12 34602, 2019
-
ruaok
yay on the shengen visa! that must be a liberating feeling!
2019-12-12 34603, 2019
-
iliekcomputers
I don't like having duplicated rabbitmq code or having to setup sentry etc
