Refactor homeserver lookup code #12

bad · 2020-10-21T19:22:46Z

bad commented

2020-10-21 19:22:46 +00:00

I find this much cleaner and more readable, but I'm bad, so code review from someone better and this please?

jboi self-assigned this 2020-10-21 20:08:04 +00:00

cadence reviewed 2020-10-22 04:45:32 +00:00

src/js/login.js Outdated

					
				@ -139,0 +138,4 @@

						} catch(e) {}

						// Find the next matrix server in the chain

						const root = await fetch(`${address}/.well-known/matrix/client`).then(res => res.json()).catch(e => {

cadence commented

2020-10-22 04:45:27 +00:00

Why chain a promise catch here? I don't have a problem with promise chaining, it's just weird considering that we used the try/catch syntax 3 lines up. IMO they should be made consistent?

bad commented

2020-10-22 05:52:18 +00:00

It was shorter. I actually wanted to write the code like it's written now but jshint errored on the optional chain. It's fixed now

bad marked this conversation as resolved

bad referenced this pull request from a commit

2020-10-23 08:57:01 +00:00

Merge pull request 'Refactor homeserver lookup code' (#12) from refactor-homeserver-lookup into princess Reviewed-on: https://gitdab.com/cadence/Carbon/pulls/12

bad merged commit 91ac7a6b3e into princess

2020-10-23 08:57:01 +00:00

bad deleted branch refactor-homeserver-lookup

2020-10-23 08:57:17 +00:00

Sign in to join this conversation.