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.

Sign up for GitHub

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

Jump to bottom

fix(xo-server/resource-set): fix error when changing VM resource set #7144

Merged
merged 6 commits into from
Nov 20, 2023
Merged

fix(xo-server/resource-set): fix error when changing VM resource set #7144

merged 6 commits into from
Nov 20, 2023

Conversation

Pizzosaure
Copy link
Contributor

@Pizzosaure Pizzosaure commented Oct 31, 2023 •
edited

Description

Fixed error displayed after successful VM addition to resource set:
value of "resourceSetId" was not properly retrieved with setData on setVmResourceSet method, so we used the barrier object to ensure that all preceding asynchronous operations are completed before proceeding to the next step.

Checklist

  • Commit
    • Title follows commit conventions
    • Reference the relevant issue (Fixes #007, See xoa-support#42, See https://...)
    • If bug fix, add Introduced by
  • Changelog
    • If visible by XOA users, add changelog entry
    • Update "Packages to release" in CHANGELOG.unreleased.md
  • PR
    • If UI changes, add screenshots
    • If not finished or not tested, open as Draft

pdonias
pdonias requested changes Oct 31, 2023
View reviewed changes
Copy link
Member

@pdonias pdonias left a comment •
edited

Choose a reason for hiding this comment

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

Commit message: the package is xo-server, not xo-web:
fix(xo-server/resource-set): fix error when changing VM resource set

@pdonias pdonias requested a review from julien-f October 31, 2023 14:55
julien-f
julien-f reviewed Oct 31, 2023
View reviewed changes
Copy link
Member

@julien-f julien-f left a comment

Choose a reason for hiding this comment

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

  1. this is not an xo-web but an xo-server fix
  2. missing changelog entry
  3. please explain the bug and how this change fixes it

@Pizzosaure Pizzosaure changed the title fix(xo-web/resourceSets): adding VM in resource set displayed error even when succeeded ix(xo-web/resource-set): fix error when changing VM resource set Oct 31, 2023
@Pizzosaure Pizzosaure changed the title ix(xo-web/resource-set): fix error when changing VM resource set fix(xo-web/resource-set): fix error when changing VM resource set Oct 31, 2023
@pdonias pdonias changed the title fix(xo-web/resource-set): fix error when changing VM resource set fix(xo-server/resource-set): fix error when changing VM resource set Nov 13, 2023
@pdonias pdonias requested review from julien-f and removed request for pdonias November 13, 2023 08:56
julien-f
julien-f reviewed Nov 13, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@pdonias pdonias requested review from julien-f and removed request for pdonias November 14, 2023 16:18
julien-f
julien-f reviewed Nov 20, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
pdonias
pdonias requested changes Nov 20, 2023
View reviewed changes
CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
@julien-f @pdonias
Update CHANGELOG.unreleased.md
16cd446 
Co-authored-by: Pierre Donias <pierre.donias@gmail.com>
@pdonias pdonias merged commit c1faaa3 into master Nov 20, 2023
1 check passed
@pdonias pdonias deleted the errorDisplayBugfix branch November 20, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julien-f julien-f julien-f left review comments

@pdonias pdonias pdonias approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Pizzosaure @julien-f @pdonias