-
Notifications
You must be signed in to change notification settings - Fork 26
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
chore: removed packngo and used metal-go apis #486
Conversation
There should be a Now that equinix-sdk-go exists, we should adopt that instead of metal-go. |
@aayushrangwala The PR test workflow is failing with the error |
@ctreatma Yes I am fixing the tests for the same. Previously there was packngo packetServer which was structured for tests/interfaces and accordingly the tests were written. Changing the structure to implement bare minimum test server and fixing tests |
@ctreatma @displague Fixed all the tests. Please review and approve if looks good |
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.
The updated tests look great. I see a couple changes that look like they might not be necessary for the migration, but not sure about that. I don't have write access to this repo in any case, so we ultimately need @cprivitere to be included in the review process as well.
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aayushrangwala, cprivitere The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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.
/lgtm
fixes: #439