-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Moving more code into shared #571
Conversation
|
What is going on with CI:
This error would make sense if the code had
Is this some Jest module mocking gone wrong or what? |
@@ -3,7 +3,6 @@ | |||
"version": "0.0.0", | |||
"private": true, | |||
"exports": { | |||
"./logger": "./dist/logger/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooof I didn't know about (or forgot about) this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking, anything to do here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was intentional. It fixed the weird logger.logger.debug
change. My comment was to Mateusz; I just meant that I'd forgot it existed so I didn't think to try deleting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continuation of PRO-720; follows up to #562
Moving much of the
replayio
utility-type code into the newshared
package so that we can use those Node APIs for querying the list of recording and uploading or deleting them.Most of this was rote but I did make a couple of changes worth calling out explicitly:
fetchAuthIdsFromGraphQL
from{ userId: string | null; workspaceId: string | null }
to{ id: string; type: "user" | "workspace" }
and updated the callers; this was easier to work with (from a types perspective)userId
orworkspaceId
though, just not bothLogger
is now eagerly instantiated; theinitialize()
method will accept a name and version and pass them along to the Grafana instance. Logging before initialization (or in Jest tests) will be local-only.package.json
(which is what I am currently doing for Puppeteer and replayio)debug(...args)
logging has been changed tologger.debug("...", {...args})
mixpanel
andlaunch-darkly
code over toshared
too, even though (for now)replayio
is the only thing using this stuffgetRecordings
method to not create so many throw-away arrays; (I didn't just do this for performance, but TypeScript was complaining about the return array containing recordings andundefined
with the previous code structure)Next steps
test-utils
dependency on@replayio/replay
(and delete@replayio/replay
)test-utils
to be a peer dependency so that it will get inlined as well (to resolve ambiguous logger ownership)logger.debug
entries and decide which ones should beinfo
,log
, or evenerror
TODO [PRO-720]
comments