"First-time contributors require mainters to approve workflows in order to run"
2021-04-23 11339, 2021
alastairp
great, that's the last thing we were missing from the switchover from jenkins
2021-04-23 11331, 2021
_lucifer
yup, right. there's a couple differences i think - its on a commit basis and its PRs from first time contributors so probably won't apply to their future PRs. but they intend to make it more flexible and configurable in future so a good start.
2021-04-23 11312, 2021
alastairp
yeah, but currently we just approve people after their first PR on jenkins anyway. So fine that it doesn't appear to future PRs
2021-04-23 11319, 2021
alastairp
the approve-per-commit is interesting
2021-04-23 11325, 2021
alastairp
similar to what ruaok mentioned when he wondered if it was possible to run CI _less often_, doesn't make sense to waste cycles if you push something only as a placeholder, and you're going to push more stuff in the future anyway
2021-04-23 11337, 2021
sumedh joined the channel
2021-04-23 11349, 2021
sumedh has quit
2021-04-23 11333, 2021
sumedh joined the channel
2021-04-23 11307, 2021
Mr_Monkey
Awesome, good to hear _lucifer
2021-04-23 11355, 2021
Mr_Monkey
Yeah source maps are going to be necessary
2021-04-23 11341, 2021
sumedh has quit
2021-04-23 11322, 2021
MRiddickW has quit
2021-04-23 11303, 2021
Mr_Monkey
_lucifer: I had a look at the sentry webpack plugin that is supposed to publish source maps to sentry on build, but to be honest I think publishing the source maps publicly is an easier solution (and I would personally benefit from having source maps available when debugging prod).
Mr_Monkey: I had also looked into that in the morning. I think we should host source maps publicly. Its easy and iirc that way we can also see them in chrome/firefox devtools/
2021-04-23 11307, 2021
Mr_Monkey
Yeah, that's it. Thanks for confirming :)
2021-04-23 11323, 2021
Mr_Monkey
I'll open a PR adding sourcemaps to our prod compilation
Front-end sentry reports coming in handy straight away :)
2021-04-23 11347, 2021
ruaok
_lucifer: I've looked at 1397 several times now and I keep getting interrupted, so feels like I've seen it many time and it looks fine to me. lol.
2021-04-23 11351, 2021
_lucifer
lol :D, regarding the test/beta comment this PR again requries DB changes so no.
2021-04-23 11321, 2021
_lucifer
if the db changes look fine I can split the PR and merge the db changes and then deploy to beta
2021-04-23 11323, 2021
ruaok
yes, but this PR does not modify existing tables, right?
2021-04-23 11309, 2021
_lucifer
the external_service_oauth is modified but that it is not currently used elsewhere.
2021-04-23 11334, 2021
ruaok
then I would say its fine to test as is.
2021-04-23 11327, 2021
_lucifer
i do not understand, should i apply the db changes?
2021-04-23 11341, 2021
ruaok
yes.
2021-04-23 11348, 2021
_lucifer
👍
2021-04-23 11301, 2021
BrainzGit
[listenbrainz-server] MonkeyDo opened pull request #1408 (master…monkey-catch-async-error-boundary): Catch unhandled promise rejections in ErrorBoundary https://github.com/metabrainz/listenbrainz-server…
2021-04-23 11337, 2021
Mr_Monkey
ishaanshah: If you remember when you implemented charts pages in LB you found a React pitfall: promise rejections aren't caught by the ErrorBoundary. A hacky solution is to throw the error inside of a setState call…
2021-04-23 11338, 2021
Mr_Monkey
Just found a good trick to avoid doing that, see above if you're interested ^
