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

fix: adds an option to select domain before profile export #2425

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

madhavilosetty-intel
Copy link
Contributor

PR Checklist

  • Unit Tests have been added for new changes
  • API tests have been updated if applicable
  • All commented code has been removed
  • If you've added a dependency, you've ensured license is compatible with Apache 2.0 and clearly outlined the added dependency.

What are you changing?

Anything the reviewer should know when reviewing this PR?

If the there are associated PRs in other repositories, please link them here (i.e. open-amt-cloud-toolkit/repo#365 )

@madhavilosetty-intel madhavilosetty-intel force-pushed the export-domain branch 7 times, most recently from 4ed1630 to 89600b1 Compare January 15, 2025 23:25
@madhavilosetty-intel
Copy link
Contributor Author

madhavilosetty-intel commented Jan 16, 2025

Please find the images showing the changes addressed in this PR.
profile screen
Profile options
Profile Key to decode

@madhavilosetty-intel madhavilosetty-intel force-pushed the export-domain branch 2 times, most recently from 8b53338 to 03311ca Compare January 17, 2025 17:36
Copy link
Member

@rsdmike rsdmike left a comment

Choose a reason for hiding this comment

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

Haven't dug too far into it quite yet, but first thing i see is that we shouldn't be prompting for domain export if they profile isnt ACM. Secondly, the dialog should preselect the first option -- since if they have an ACM profile, a domain MUST be selected. Lastly, if there are no domains upon trying to export an ACM profile, then the dialog should NOT come up and an error should be displayed that they must add a domain first before being able to export/use the profile.

@madhavilosetty-intel madhavilosetty-intel force-pushed the export-domain branch 3 times, most recently from 4c7f316 to 7d505ae Compare January 30, 2025 21:39
@madhavilosetty-intel madhavilosetty-intel enabled auto-merge (squash) January 30, 2025 21:40
@madhavilosetty-intel madhavilosetty-intel force-pushed the export-domain branch 2 times, most recently from d494fec to cdceee7 Compare January 30, 2025 21:53
@madhavilosetty-intel madhavilosetty-intel merged commit e80b424 into main Jan 30, 2025
8 checks passed
@madhavilosetty-intel madhavilosetty-intel deleted the export-domain branch January 30, 2025 22:46
RosieAMT pushed a commit that referenced this pull request Jan 30, 2025
## [3.30.1](v3.30.0...v3.30.1) (2025-01-30)

### Bug Fixes

* adds an option to select domain before profile export ([#2425](#2425)) ([e80b424](e80b424))
@RosieAMT
Copy link

🎉 This PR is included in version 3.30.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants