@bartek, if I missed it, can you please link the diff where this is handled? On the other hand, if it's not handled yet, can you share how you're keeping track of each callsite that needs to be migrated?

lib/actions/upload-actions.js
36	How was this not causing ESLint errors earlier? Something seems sketchy in this stack... saw the same thing earlier here. @bartek, can you please explain what you were doing here, and how you managed to get earlier diffs to pass ESLint even though there were clear issues like this? Perhaps you have commits in your branch that you haven't diffed up? If so, please squash those and update all of the diffs. And separately, please make sure to apply fixes like this to the correct commits in the future, instead of putting them wherever is convenient for you.

This revision now requires changes to proceed.Mon, Nov 25, 5:40 AM

bartek added inline comments.Mon, Nov 25, 7:10 AM

lib/actions/upload-actions.js
36	This was my mistake. When resolving conflicts, pre-commit hooks don't apply. I put the fixup commit to move the import to another diff, and updated both diffs locally. Then I published only the other diff without putting rebased revision for this diff. I'll rebase the whole stack to make sure everything is correct now. And separately, please make sure to apply fixes like this to the correct commits in the future, instead of putting them wherever is convenient for you. I'll keep that in mind, thank you

ashoat added inline comments.Mon, Nov 25, 1:22 PM

lib/actions/upload-actions.js
36	Thanks :)

bartek mentioned this in D13959: [lib] Extract function to assign holder.Tue, Nov 26, 4:10 AM

Rebase, handle getKeyserverOverrideForAnInviteLink better.

In D13989#390996, @ashoat wrote:

getKeyserverOverrideForAnInviteLink does not appear to be handled, even though it was extracted in D13958.

Initially I was thinking that this could be handled the same way as error case so the function should return null. After digging deeper into the invite links code I realized this was wrong and it should also be handled separately.

Harbormaster completed remote builds in B32841: Diff 46056.Tue, Nov 26, 4:56 AM

Thanks for handling getKeyserverOverrideForAnInviteLink!

Would mind sharing how you're keeping track of each callsite that needs to be migrated? I'm worried because I caught two cases in my review, but I didn't do an exhaustive analysis, and so I'm worried there are other cases I might not have caught.

This revision is now accepted and ready to land.Tue, Nov 26, 5:22 AM

Answered in https://phab.comm.dev/D13992#391425

Closed by commit rCOMM40d00d2d189e: [lib][keyserver] Handle invalid CSAT on blob download (authored by bartek). · Explain WhyFri, Nov 29, 5:03 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM40d00d2d189e: [lib][keyserver] Handle invalid CSAT on blob download.