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

feat: [M3-5561] - Add Private IP checkbox when cloning a Linode #9039

Merged
merged 10 commits into from
Apr 25, 2023

Conversation

cpathipa
Copy link
Contributor

@cpathipa cpathipa commented Apr 20, 2023

Description 📝

Added Private IP checkbox when cloning a Linode.
Unit test coverage for AddonsPanel

Preview 📷

Include a screenshot or screen recording of the change

Before After
image image

How to test 🧪

  • Navigate to create Linode
  • Create Linode make sure to check private_ip address.
  • Clone the Linode, Validate private_ip address checkbox.
  • The checkbox should default to being checked if the source Linode has a Private IP allocated to it.
  • yarn test
  • yarn cy:run

@cpathipa cpathipa self-assigned this Apr 20, 2023
@cpathipa cpathipa marked this pull request as draft April 20, 2023 16:56
@cypress
Copy link

cypress bot commented Apr 20, 2023

Passing run #3167 ↗︎

0 151 3 0 Flakiness 0

Details:

Merge remote-tracking branch 'origin/develop' into M3-5561
Project: Cloud Manager E2E Commit: eacc61e80c
Status: Passed Duration: 13:28 💡
Started: Apr 25, 2023 11:20 AM Ended: Apr 25, 2023 11:33 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@cpathipa cpathipa marked this pull request as ready for review April 21, 2023 00:57
}

const AddonsPanel = (props: Props) => {
export type AddonsPanelProps = Props;
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to do an alias here? Could we just name Props AddonsPanelProps?

@cpathipa cpathipa added the Add'tl Approval Needed Waiting on another approval! label Apr 21, 2023
Copy link
Contributor

@dwiley-akamai dwiley-akamai left a comment

Choose a reason for hiding this comment

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

Functionality looks good 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we make use of factories in this file instead of hardcoding all the mocked data?

Copy link
Contributor

@hkhalil-akamai hkhalil-akamai left a comment

Choose a reason for hiding this comment

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

Left some minor feedback, otherwise LGTM!

@hkhalil-akamai hkhalil-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Apr 22, 2023
@cpathipa cpathipa merged commit e067b0f into linode:develop Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants