ENG-2049 Refactor publishNodesToGroup to use the crossAppNodeSchema#1244
ENG-2049 Refactor publishNodesToGroup to use the crossAppNodeSchema#1244maparent wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
ebba3ac to
0744f8f
Compare
| groupIds: string[]; | ||
| publishNodes: PublishNode[]; | ||
| }): Promise<PublishNodesResult> => { | ||
| const nodes = await publishNodesToCrossApp(publishNodes); |
There was a problem hiding this comment.
which will also rename this to nodeUidWithTypeToCrossApp
0744f8f to
3c097e7
Compare
3c097e7 to
51ca3bc
Compare
51ca3bc to
dc87ee0
Compare
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Let's fix the CI then we can merge
| spaceId: context.spaceId, | ||
| groupIds: selectedGroupIds, | ||
| nodes: publishableNodes, | ||
| publishNodes: publishableNodes, |
There was a problem hiding this comment.
this could probably stay as nodes
| client, | ||
| spaceId, | ||
| groupIds, | ||
| publishNodes, |
There was a problem hiding this comment.
this could probably just be nodes
There was a problem hiding this comment.
Using nodeUids; using nodes interferes with naming inside the function, and I think nodes is giving the misleading impression that there is more information than there is.
Could use nodeUidsWithType if you find it clearer; I find it needlessly verbose.
There was a problem hiding this comment.
If you're ok with that name it's otherwise ready to merge
2 participants
ENG-2049
https://www.loom.com/share/908e003a749141fcb5d25846018a48eb
Tested with a publication.