Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
starcke
force-pushed
the
starcke/fetch-queries-out-of-view
branch
from
b9d1bb9 to
9543ab3
Compare
| import { redactableError } from "../common/errors"; | ||
| import { readQueryResults, runQuery } from "./external-api-usage-queries"; | ||
| import { | ||
| readQueryResults, |
There was a problem hiding this comment.
Should this also be renamed? e.g. readExternalApiQueryResults
There was a problem hiding this comment.
I think it is not used in the view anymore, so I think it makes sense to just be readQueryResults. But I am also happy to rename it.
There was a problem hiding this comment.
Oh why is it imported then? 🤔
There was a problem hiding this comment.
Sorry I can't read a diff right 🤦 it looks like we can remove the export from that function though.
There was a problem hiding this comment.
I had problems with the test not being able to call it then, but maybe I am missing something?
There was a problem hiding this comment.
Ah, no you're not! So technically the function shouldn't be exported because it's not used by other places in the extension code. And tests shouldn't be testing that directly, but ideally should go through the entry point to the module rather than this function.
That might be a bit of a faff atm though so I'm happy for us to take it on at tech debt.
3 participants
Replace this with a description of the changes your pull request makes.
Checklist
ready-for-doc-reviewlabel there.