Skip to content

Conversation

@DaanHoogland
Copy link
Contributor

@DaanHoogland DaanHoogland commented Nov 1, 2023

Description

This PR Fixes: #7864

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

debugged in local UI against lab environment.
Note that this is a pure UI change but is dependend on the template being for vmware and thus can not be tested in the qa env (which is simulator only).

@DaanHoogland DaanHoogland added this to the 4.18.2.0 milestone Nov 1, 2023
@DaanHoogland DaanHoogland requested a review from shwstppr November 1, 2023 16:32
@DaanHoogland
Copy link
Contributor Author

@blueorangutan ui

@blueorangutan
Copy link

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@codecov
Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Merging #8173 (fbc94bd) into 4.18 (3b11663) will increase coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               4.18    #8173   +/-   ##
=========================================
  Coverage     13.06%   13.07%           
- Complexity     9111     9113    +2     
=========================================
  Files          2720     2720           
  Lines        257592   257592           
  Branches      40158    40158           
=========================================
+ Hits          33663    33674   +11     
+ Misses       219698   219685   -13     
- Partials       4231     4233    +2     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@DaanHoogland
Copy link
Contributor Author

@blueorangutan ui

@blueorangutan
Copy link

@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/8173 (QA-JID-218)

@DaanHoogland
Copy link
Contributor Author

@andrijapanicsb , low prio but you want this! ;)

Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

Change at line 231 looks good. Rest seems refactoring

@DaanHoogland
Copy link
Contributor Author

Change at line 231 looks good. Rest seems refactoring

that's right @shwstppr , but the renaming of values to formvalues is because of a name clash that i encountered debugging. The fetchNicAdapterTypes() changes are pure refactoring.

@DaanHoogland DaanHoogland marked this pull request as ready for review November 2, 2023 09:01
Copy link
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

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

LGTM - didn't test it though

@rajujith
Copy link

rajujith commented Nov 8, 2023

@blueorangutan package

@blueorangutan
Copy link

@rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7674

Copy link

@rajujith rajujith left a comment

Choose a reason for hiding this comment

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

LGTM. Verified that the NIC Adapter setting 'vmxnet3' input in the template registration form is retained in the template settings.

@shwstppr shwstppr merged commit e790047 into apache:4.18 Nov 9, 2023
@DaanHoogland DaanHoogland deleted the 7864-vmware-adapter-detail-not-passed-during-registering-template branch November 9, 2023 09:29
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants