-
Notifications
You must be signed in to change notification settings - Fork 0
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
v2 #2
v2 #2
Conversation
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.
Fantastic first PR 🎉
Super well documented and clear - great to see.
Nice find on pagination / cursors - makes total sense as the cause of our issues.
A few nitpicks, and some comments on how to handle types, but zero showstoppers here. Looking forward to trying it out 👍
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.
Actioned all PR comments. I think this is ready to roll but it could do with a final review!
(in the meantime I've tagged the latest comment as v2.0
in order to do one more round of testing)
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.
Changes look great! A couple very minor questions/comments on final read through, but no blockers - excited to get v2 merged & in use over in planx-new ✨
…ve README, add missing types for vultr-node, rebuild dist accept operating system input by name (os_type) rather than Vultr ID
This PR does the following:
I initially put this logic in
planx-new
(see theopensystemslab/planx-new#2959) but later realised that was the wrong move, and bundled it over here. This should also mean fewer changes to the github workflows.