-
Notifications
You must be signed in to change notification settings - Fork 17
Conversation
2ff54d2
to
4300c34
Compare
4300c34
to
93dd846
Compare
(I don't have permissions here, so you'll need to ask Mark or someone else to do a review). |
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.
I've commented on a few things I've noted. We should definitely fix the environment variable names, the other comments are mostly suggestions. I'm not super familiar with the codebase of sync-team and others can probably give better feedback on the implementation, but it looks reasonable. 👍
93dd846
to
7758a61
Compare
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.
Looks reasonable to me, with the limited knowledge that I have of the sync-team
codebase. 🙈
The logs after we merged this look identical to the previous run 👍 |
🎉 🎉 🎉 Thank you! |
No description provided.