17:19 PM
reosarevok
yvanzo: ticket editing permissions seem too important to restrict IMO
2019-07-24 20533, 2019
17:19 PM
reosarevok
So I guess we need to live with users changing priorities
2019-07-24 20542, 2019
17:19 PM
reosarevok
Can we follow priority changes exclusively?
2019-07-24 20532, 2019
17:20 PM
akhilesh
Mr_Monkey: yes, thanks for resolving that issue.
2019-07-24 20532, 2019
17:21 PM
bitmap
Cyna: make sure req is being outputted by the sanitizedContext function, we went over this in another branch
2019-07-24 20539, 2019
17:23 PM
Cyna
I made some changes, removed the uses of $c in the file that is being hydrated
2019-07-24 20551, 2019
17:23 PM
Cyna
Replaced by directly passing in params
2019-07-24 20522, 2019
17:24 PM
Cyna
`TypeError: $data.selectionMessage is not a function` getting this in the console now
2019-07-24 20526, 2019
17:27 PM
reosarevok
bitmap, yvanzo: do you want to talk PRs then? :)
2019-07-24 20529, 2019
17:27 PM
bitmap
reosarevok, yvanzo: I'm around, just making coffee first
2019-07-24 20543, 2019
17:27 PM
bitmap
sure :)
2019-07-24 20515, 2019
17:28 PM
yvanzo
2019-07-24 20503, 2019
17:29 PM
yvanzo
(there is no notification for field-level change)
2019-07-24 20529, 2019
17:34 PM
alastairp
aidanlw17: Cyna: please put your flight numbers on the wiki for the summit so we can coordinate arrival times
2019-07-24 20500, 2019
17:39 PM
Cyna
Well My flights haven't been booked yet, As the VISA process is still pending
2019-07-24 20529, 2019
17:42 PM
bitmap
2019-07-24 20538, 2019
17:43 PM
alastairp
Cyna: great, as soon as you know then :)
2019-07-24 20551, 2019
17:43 PM
bitmap
seems to have no conflicts, so should be in a good state. would it help to try doing the code review on irc?
2019-07-24 20521, 2019
17:44 PM
reosarevok
yvanzo: ^ :) (works fo rme)
2019-07-24 20550, 2019
17:44 PM
pristine__
alastairp: I need help in defining a config var name.
2019-07-24 20503, 2019
17:45 PM
bitmap
maybe asking "why is line x doing y" or "i'm not sure about z" here would be faster than back-and-forth on github
2019-07-24 20504, 2019
17:46 PM
pristine__
We have a date time value = x and we wish to move date in x to a particular date, something like, date.replace(days=y)
2019-07-24 20529, 2019
17:46 PM
pristine__
What should be the name of y ?
2019-07-24 20537, 2019
17:46 PM
pristine__
Y is a integer here.
2019-07-24 20543, 2019
17:46 PM
pristine__
an*
2019-07-24 20549, 2019
17:47 PM
Rotab joined the channel
2019-07-24 20526, 2019
17:48 PM
yvanzo
bitmap, reosarevok: Rule added.
2019-07-24 20531, 2019
17:48 PM
bitmap has the branch checked out too
2019-07-24 20537, 2019
17:48 PM
yvanzo too
2019-07-24 20553, 2019
17:48 PM
Cyna
bitmap: `TypeError: $data.selectionMessage is not a function` what does this mean ?
2019-07-24 20501, 2019
17:49 PM
Cyna
dependency issue ?
2019-07-24 20512, 2019
17:49 PM
Cyna
I've ruled out that its a syntax error
2019-07-24 20535, 2019
17:49 PM
pristine__
ruaok can also help:)
2019-07-24 20557, 2019
17:50 PM
bitmap
Cyna: search for area_bubble root/components/forms.tt. it must not be applying the knockout bindings correctly ($data should be an area entity)
2019-07-24 20505, 2019
17:51 PM
yvanzo
bitmap, reosarevok: I actually started to review it today, shouldn’t be long to finish, can we jump to the next one?
2019-07-24 20514, 2019
17:51 PM
bitmap
okay
2019-07-24 20530, 2019
17:51 PM
bitmap
2019-07-24 20543, 2019
17:51 PM
reosarevok
I'll quickly rebase
2019-07-24 20556, 2019
17:51 PM
reosarevok
Not that the conflict is very relevant, but
2019-07-24 20512, 2019
17:52 PM
bitmap
we haven't added any new NotFound pages since, I hope
2019-07-24 20541, 2019
17:52 PM
yvanzo
2019-07-24 20542, 2019
17:52 PM
BrainzBot
MBS-9911: Convert attributes admin templates to React/JSX
2019-07-24 20539, 2019
17:54 PM
bitmap
I guess it should be easy to see from components.js after you rebase
2019-07-24 20502, 2019
17:55 PM
reosarevok
bitmap: we added genre but IIRC I already updated it for that
2019-07-24 20523, 2019
17:55 PM
bitmap
okay, good
2019-07-24 20530, 2019
17:57 PM
reosarevok
Rebased, pushed
2019-07-24 20536, 2019
17:57 PM
yvanzo
I don’t really the added value of refactoring all NotFound in a single file, there is almost no code deduplication here, am I missing something?
2019-07-24 20509, 2019
17:58 PM
reosarevok
They're all the same concept, with mostly small variations on one string
2019-07-24 20516, 2019
17:58 PM
reosarevok
It makes much more sense to me for all of it to be together
2019-07-24 20501, 2019
17:59 PM
reosarevok
(if we decide to change something of how it works, we can change one file rather than 30)
2019-07-24 20535, 2019
17:59 PM
Cyna
yvanzo: done
2019-07-24 20519, 2019
18:00 PM
aidanlw17
alastairp: I’ll put mine up right now!
2019-07-24 20529, 2019
18:00 PM
reosarevok
Cyna: thanks! yvanzo: works nicely!
2019-07-24 20513, 2019
18:01 PM
bitmap
the NotFound pages all previously shared the same structure, so they had the same imports and markup, just the messages differed
2019-07-24 20549, 2019
18:01 PM
yvanzo
We could have the rule to automatically restore the previously set priority with an explanatory comment but I feel we should preferably review it.
2019-07-24 20553, 2019
18:01 PM
bitmap
so not much actual code/logic was duplicated, but a lot of boilerplate (imports/markup)
2019-07-24 20551, 2019
18:02 PM
alastairp
pristine__: can you pastebin the code with a bit more context?
2019-07-24 20554, 2019
18:02 PM
reosarevok
Yeah, I agree that if it's not crazy (and it shouldn't be) we should review it :)
2019-07-24 20510, 2019
18:03 PM
reosarevok
I suspect around half are just pointless changes, but half are relevant
2019-07-24 20513, 2019
18:04 PM
Cyna
bitmap: the bindings werent applied because I didnt add ```<!-- ko with: target() && target().area -->
2019-07-24 20513, 2019
18:04 PM
Cyna
{...}
2019-07-24 20513, 2019
18:04 PM
Cyna
<!-- /ko -->```
2019-07-24 20550, 2019
18:04 PM
Cyna
Its strange adding something like this to React
2019-07-24 20553, 2019
18:04 PM
Cyna
So I left it out
2019-07-24 20506, 2019
18:05 PM
Cyna
Dont know how to make it work, cause it doesnt work directly
2019-07-24 20528, 2019
18:05 PM
yvanzo
Well, I don’t think it hurts.
2019-07-24 20545, 2019
18:05 PM
pristine__
2019-07-24 20559, 2019
18:05 PM
pristine__
line 19
2019-07-24 20504, 2019
18:06 PM
reosarevok
yvanzo: the priority thing or the PR?
2019-07-24 20528, 2019
18:06 PM
pristine__
I don't think passing "1" is a good idea. I want to define this in config
2019-07-24 20535, 2019
18:06 PM
pristine__
alastairp: ^
2019-07-24 20500, 2019
18:10 PM
pristine__
ruaok: How have we configured the three workers. I don't understand it through the docker compose. On my local machine, I have one master container and one worker container as reflected in docker compose, and the same docker compose is used on leader, then how three worker containers?
2019-07-24 20520, 2019
18:10 PM
yvanzo
reosarevok: the PR
2019-07-24 20501, 2019
18:11 PM
aidanlw17
2019-07-24 20507, 2019
18:11 PM
reosarevok
yvanzo: ok, other than that is there something in the code to comment etc? :)
2019-07-24 20511, 2019
18:11 PM
reosarevok
aidanlw17: it's a wiki ;)
2019-07-24 20519, 2019
18:12 PM
aidanlw17
reosarevok: just dunno if it's ok for me to change the schedule? guessing that's a yes ;)
2019-07-24 20527, 2019
18:12 PM
reosarevok
If it's a clear error, sure
2019-07-24 20534, 2019
18:12 PM
reosarevok
(otherwise, suggest only maybe :) )
2019-07-24 20544, 2019
18:12 PM
reosarevok
yvanzo: I'll take that as a no :)
2019-07-24 20546, 2019
18:12 PM
aidanlw17
Ok, thanks!
2019-07-24 20551, 2019
18:12 PM
reosarevok
Oh, I saw the comment now
2019-07-24 20500, 2019
18:13 PM
yvanzo
reosarevok: sorry, approved on github already
2019-07-24 20504, 2019
18:13 PM
bitmap
nice, wanna do a couple more?
2019-07-24 20529, 2019
18:13 PM
reosarevok
Works for me
2019-07-24 20540, 2019
18:13 PM
reosarevok
I need hours this week anyway, I've been too lazy :p
2019-07-24 20558, 2019
18:13 PM
bitmap
2019-07-24 20520, 2019
18:14 PM
reosarevok
Oh! :)
2019-07-24 20535, 2019
18:14 PM
reosarevok
I guess the comment is a good one in that the variable is not very clearly named
2019-07-24 20542, 2019
18:14 PM
reosarevok
Will rebase + rename!
2019-07-24 20519, 2019
18:15 PM
bitmap
though with xByY naming I usually think it's an object where you can lookup an x using y as a key
2019-07-24 20529, 2019
18:15 PM
yvanzo
+ TODOs should probably be better named/placed to be meaningful outside of the PR.
2019-07-24 20521, 2019
18:16 PM
reosarevok
bitmap: any suggestions that make it more clear?
2019-07-24 20529, 2019
18:16 PM
bitmap
maybe tracksWith instead of tracksBy?
2019-07-24 20537, 2019
18:16 PM
reosarevok
Maybe :)
2019-07-24 20546, 2019
18:16 PM
yvanzo
+1
2019-07-24 20520, 2019
18:18 PM
bitmap
for TODO: RecordingT should have an optional artistCredit property
2019-07-24 20527, 2019
18:18 PM
prashanth041 joined the channel
2019-07-24 20501, 2019
18:19 PM
bitmap
it makes it sound like we should change RecordingT, but it should only be optional through recordings
2019-07-24 20518, 2019
18:19 PM
bitmap
I mean tracks, sorry
2019-07-24 20555, 2019
18:19 PM
bitmap
as in track.recording should be something like RecordingWithOptionalArtistCreditT :)
2019-07-24 20513, 2019
18:21 PM
reosarevok
Is there a way to say "RecordingT, but with this one difference" without defining it all again
2019-07-24 20515, 2019
18:21 PM
reosarevok
?
2019-07-24 20522, 2019
18:21 PM
reosarevok
If there is, we could just change it now I guess
2019-07-24 20542, 2019
18:22 PM
bitmap
maybe $ReadOnly<{|...RecordingT, artistCredit?: ArtistCreditT|}> ?
2019-07-24 20551, 2019
18:22 PM
bitmap
haven't tested it
2019-07-24 20503, 2019
18:23 PM
reosarevok
"Test" means "run flow, see if it screams", right? Can do
2019-07-24 20513, 2019
18:23 PM
bitmap
but I think later properties should override old ones in spreads like that
2019-07-24 20556, 2019
18:23 PM
reosarevok
No errors, so I guess it's fine
2019-07-24 20501, 2019
18:25 PM
reosarevok
For the other, is it enough to give " // TODO: cdtocs needs to be defined eventually: +cdtocs: $ReadOnlyArray<MediumCdTocT>" ? yvanzo?
2019-07-24 20510, 2019
18:25 PM
reosarevok
Or "TODO: still missing cdtocs: etc"?
2019-07-24 20512, 2019
18:25 PM
yvanzo
isn't it "recording" attribute rather than "RecordingT" here that should have optional "artistCredit"?
2019-07-24 20557, 2019
18:25 PM
yvanzo
(but that's better if you fixed it rather than keeping a TODO :)
2019-07-24 20559, 2019
18:25 PM
reosarevok
What's the difference? The recording attribute is a RecordingT, right? So it's "a RecordingT but with optional artistCredit
2019-07-24 20501, 2019
18:26 PM
bitmap
yup
2019-07-24 20536, 2019
18:26 PM
yvanzo
reosarevok: no worry, I was just typing about the comment while you were fixing the real issue.
2019-07-24 20538, 2019
18:26 PM
bitmap
the difference is just the wording, adding attribute would make it clear you're not talking about the type
2019-07-24 20549, 2019
18:26 PM
reosarevok
Oh!
2019-07-24 20558, 2019
18:26 PM
reosarevok
Gotcha
2019-07-24 20522, 2019
18:28 PM
bitmap
though I'm not sure my suggestion actually makes the property optional
2019-07-24 20538, 2019
18:29 PM
bitmap
this seems to {+artistCredit?: ArtistCreditT} & RecordingT
2019-07-24 20553, 2019
18:30 PM
yvanzo
the other TODO wording is fine although I'm not sure it is necessary and correct (shouldn't cdtoc be optional in some case?)
2019-07-24 20542, 2019
18:31 PM
bitmap
I'm not sure they're always loaded, but we always output a cdtocs array in Medium::TO_JSON
2019-07-24 20507, 2019
18:32 PM
bitmap
so it would just be an empty array if they're not loaded
2019-07-24 20530, 2019
18:32 PM
yvanzo
OK then!
2019-07-24 20507, 2019
18:34 PM
reosarevok
IS something like this better?
2019-07-24 20509, 2019
18:34 PM
yvanzo
Is there a way to check that JSON passed to the renderer matches flow types at run time?
2019-07-24 20510, 2019
18:34 PM
reosarevok
2019-07-24 20512, 2019
18:35 PM
bitmap
2019-07-24 20529, 2019
18:35 PM
yvanzo
reosarevok: please add 💿 💿 💿 ;)
2019-07-24 20532, 2019
18:36 PM
bitmap
$ReadOnlyArray<💿>
2019-07-24 20540, 2019
18:37 PM
reosarevok
Ok, more seriously, I'll push
2019-07-24 20558, 2019
18:38 PM
yvanzo
2019-07-24 20559, 2019
18:38 PM
BrainzBot
MBS-10298: Check flow type at run time
2019-07-24 20515, 2019
18:39 PM
bitmap
thx
2019-07-24 20547, 2019
18:39 PM
alastairp
pristine__: hmm, a few ideas here
2019-07-24 20535, 2019
18:40 PM
alastairp
how many times do you use this method replace_days? if it's only to get the first of the month, then I'd name it something like get_first_of_this_month instead
2019-07-24 20541, 2019
18:40 PM
alastairp
in the case that you have at the moment, `days` is ok
2019-07-24 20550, 2019
18:40 PM
alastairp
however, in this case I wouldn't do it like this
2019-07-24 20504, 2019
18:41 PM
alastairp
instead, I'd pass in the year/month to create_training_data_from_window
2019-07-24 20508, 2019
18:41 PM
yvanzo
2019-07-24 20518, 2019
18:41 PM
alastairp
this way you could theoretically calculate the data for any month
2019-07-24 20540, 2019
18:41 PM
prashanth041 has quit
2019-07-24 20506, 2019
18:42 PM
reosarevok
2019-07-24 20508, 2019
18:42 PM
alastairp
normally it's better to pass information like this (what month to act on) as an argument, and collect the data as close to the "user" as possible
2019-07-24 20558, 2019
18:42 PM
alastairp
e.g. if it's called by a user or by cron, either pass in this information as an argument, or in the main function get the current month and then pass that into your function
2019-07-24 20542, 2019
18:43 PM
yvanzo
reosarevok: OK, thanks, would be nice to explain that we had both host and Referer in the commit message
2019-07-24 20505, 2019
18:44 PM
reosarevok
Can do, how would you word it?
2019-07-24 20510, 2019
18:44 PM
reosarevok
bitmap: ? :)
2019-07-24 20554, 2019
18:45 PM
bitmap
well, fixing the case allows us to access them consistently
