Skip to content

Implement ID fields for StackTrace and Callframes#54

Merged
josephburgess merged 9 commits intotesterloop:masterfrom
josephburgess:adding-ID-Node-StackTrace
Jun 7, 2023
Merged

Implement ID fields for StackTrace and Callframes#54
josephburgess merged 9 commits intotesterloop:masterfrom
josephburgess:adding-ID-Node-StackTrace

Conversation

@josephburgess
Copy link
Copy Markdown
Member

No description provided.

meiamsome
meiamsome previously approved these changes May 25, 2023
@josephburgess josephburgess force-pushed the adding-ID-Node-StackTrace branch from 528a818 to 0db7c13 Compare May 30, 2023 11:49
@josephburgess josephburgess marked this pull request as ready for review May 30, 2023 11:55
@josephburgess josephburgess requested a review from meiamsome May 30, 2023 15:35
@josephburgess josephburgess changed the title Implementing ID fields for StackTrace and Callframes Implement ID fields for StackTrace and Callframes May 31, 2023
Comment thread src/resolvers/ConsoleLogEvent.ts Outdated
Comment thread src/schema/execution/console/StackTrace.graphql Outdated
Comment thread src/resolvers/ConsoleLogEvent.ts Outdated
Comment on lines 42 to 46
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually a bit anti-patterny in the old code, we should probably move this to the StackTrace.callFrames resolver, you will need add a mapper for StackFrame that is just id based.

@meiamsome meiamsome dismissed their stale review June 5, 2023 04:18

outdated

@josephburgess josephburgess requested a review from meiamsome June 5, 2023 09:19
meiamsome
meiamsome previously approved these changes Jun 5, 2023
meiamsome
meiamsome previously approved these changes Jun 5, 2023
meiamsome
meiamsome previously approved these changes Jun 5, 2023
Comment thread src/resolvers/types/mappers.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth changing this to consoleLogEvent: ConsoleLogEventModel to make it clearer what's going on?

@josephburgess josephburgess force-pushed the adding-ID-Node-StackTrace branch from 43b0863 to b6fe319 Compare June 6, 2023 17:01
@josephburgess josephburgess merged commit c1d480d into testerloop:master Jun 7, 2023
@josephburgess josephburgess deleted the adding-ID-Node-StackTrace branch June 7, 2023 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants