"First-time contributors require mainters to approve workflows in order to run"
great, that's the last thing we were missing from the switchover from jenkins
_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.
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
the approve-per-commit is interesting
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
sumedh joined the channel
sumedh has quit
sumedh joined the channel
Mr_Monkey
Awesome, good to hear _lucifer
Yeah source maps are going to be necessary
sumedh has quit
MRiddickW has quit
_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/
Mr_Monkey
Yeah, that's it. Thanks for confirming :)
I'll open a PR adding sourcemaps to our prod compilation
Front-end sentry reports coming in handy straight away :)
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.
_lucifer
lol :D, regarding the test/beta comment this PR again requries DB changes so no.
if the db changes look fine I can split the PR and merge the db changes and then deploy to beta
ruaok
yes, but this PR does not modify existing tables, right?
_lucifer
the external_service_oauth is modified but that it is not currently used elsewhere.
ruaok
then I would say its fine to test as is.
_lucifer
i do not understand, should i apply the db changes?
ruaok
yes.
_lucifer
đź‘Ť
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-serv...
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…
Just found a good trick to avoid doing that, see above if you're interested ^
