Skip to content
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

update: Additional drive action structuring and extraction #911

Merged
merged 3 commits into from
Jan 22, 2025

Conversation

isTravis
Copy link
Member

@isTravis isTravis commented Jan 22, 2025

High-level Explanation of PR

This addresses many (all?) of the remaining migration changes needed to support GDrive editing.

Test Plan

I think I'll need to test this against our prod Arcadia community to get a real sense of whether anything is missing. (Would be great to be able to clone a prod community to dev at some point). But, I only touch Drive action files, so I don't think the surface area is too large if I did miss a bug somewhere.

@isTravis isTravis requested a review from gabestein January 22, 2025 17:06
@@ -158,6 +157,15 @@ export const run = defineRun<typeof action>(
relations,
});
}
await updatePub({
Copy link
Member Author

@isTravis isTravis Jan 22, 2025

Choose a reason for hiding this comment

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

This is my first time using updatePub in an action (as opposed to just creating new related pubs). Am I missing anything obvious with how I setup the call?

Copy link
Member

Choose a reason for hiding this comment

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

nope, looks good to me. that looks like the right value format and passing on the lastModifiedBy like you did is correct

@isTravis isTravis requested a review from kalilsn January 22, 2025 17:25
Copy link
Member

@kalilsn kalilsn left a comment

Choose a reason for hiding this comment

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

i haven't run this since i don't know how but i think it's fine to merge! and the tests look good!

@isTravis isTravis merged commit 730195c into main Jan 22, 2025
6 checks passed
@isTravis isTravis deleted the tr/migrationUpdates-01-22 branch January 22, 2025 18:02
@isTravis isTravis restored the tr/migrationUpdates-01-22 branch January 22, 2025 18:02
@isTravis isTravis deleted the tr/migrationUpdates-01-22 branch January 22, 2025 18:02
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