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 zerto delete plan error #1973

Merged
merged 2 commits into from
Feb 3, 2020
Merged

Fix zerto delete plan error #1973

merged 2 commits into from
Feb 3, 2020

Conversation

JeremyDec
Copy link
Contributor

MANAGER-4207

@JeremyDec JeremyDec added the bug Something isn't working label Dec 18, 2019
FredericEspiau
FredericEspiau previously approved these changes Dec 18, 2019
View reviewed changes
@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from c67279e to 79167b2 Compare December 23, 2019 09:24
@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch 3 times, most recently from d4ddda6 to 3d026ee Compare December 30, 2019 09:45
@antleblanc
Copy link
Member

Hi @JeremyDec!

We recently merged #1336 which apply Prettier rules so could you please rebase
your branch on develop?

To easily resolve conflicts you can ignore HEAD changes and only keep yours.

After that you can run:

$ yarn run lint

This command will lint the entire codebase.

or:

Amend your commit to trigger the pre-commit hook (prettier will be launched
under the hood).

Thanks!

@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from 3d026ee to 383f576 Compare January 2, 2020 08:31
@JeremyDec
Copy link
Contributor Author

@antleblanc done

@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from 383f576 to fd2bcc6 Compare January 7, 2020 10:25
@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from fd2bcc6 to 89c81b2 Compare January 8, 2020 13:04
@ovh ovh deleted a comment from ovh-cds Jan 9, 2020
@JeremyDec
Copy link
Contributor Author

@marie-j / @FredericEspiau, your comments have been taken in account

@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from 54f962a to d370e60 Compare January 17, 2020 16:36
@JeremyDec JeremyDec requested a review from antleblanc January 17, 2020 16:37
@JeremyDec JeremyDec dismissed antleblanc’s stale review January 17, 2020 16:37

Review taken in account

FredericEspiau
FredericEspiau previously approved these changes Jan 20, 2020
View reviewed changes
@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from d370e60 to 246105d Compare January 24, 2020 13:31
@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch 2 times, most recently from 4b008b5 to 7e01553 Compare January 27, 2020 14:54
@JeremyDec JeremyDec force-pushed the fix/drp-delete-error-message branch from 7e01553 to 47433e5 Compare January 29, 2020 09:12
@antleblanc antleblanc self-requested a review January 29, 2020 15:33
FredericEspiau
FredericEspiau previously approved these changes Jan 30, 2020
View reviewed changes
marie-j
marie-j previously approved these changes Jan 30, 2020
View reviewed changes
.../dedicated/client/app/dedicatedCloud/datacenter/drp/dedicatedCloud-datacenter-drp.routing.js
'dedicatedCloud_datacenter_drp_confirm_create_error',
)} ${get(error, 'data.message', error.message)}`,
);
}),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This functions looks a bit big. Can it be chunked into smaller parts ? (I understand that some logic is linked to display so not everything can go to a service

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right it is big. What could be splitted, is the logic related to user preferences. But as I can't set the whole function into a service (with display logic as you raised the point), I choose to let it there

antleblanc
antleblanc previously approved these changes Feb 3, 2020
View reviewed changes
@JeremyDec @antleblanc
fix(dedicatedcloud.datacenter.drp): fix use of preference key
ad4b019 
ref: MANAGER-4207

Signed-off-by: Jérémy De-Cesare <[email protected]>
@JeremyDec @antleblanc
refactor(dedicatedcloud.datacenter.drp): minor improvements
887e797 
factorize method and clean useless injections

Signed-off-by: Jérémy De-Cesare <[email protected]>
@antleblanc antleblanc dismissed stale reviews from marie-j, FredericEspiau, and themself via 887e797 February 3, 2020 10:54
@antleblanc antleblanc force-pushed the fix/drp-delete-error-message branch from 47433e5 to 887e797 Compare February 3, 2020 10:54
@antleblanc antleblanc self-requested a review February 3, 2020 10:54
@antleblanc antleblanc merged commit 75f6b7c into develop Feb 3, 2020
@antleblanc antleblanc deleted the fix/drp-delete-error-message branch February 3, 2020 10:55
@antleblanc antleblanc mentioned this pull request Feb 3, 2020
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbourgois cbourgois cbourgois left review comments

@antleblanc antleblanc antleblanc approved these changes

@FredericEspiau FredericEspiau FredericEspiau left review comments

@marie-j marie-j marie-j left review comments

@AxelPeter AxelPeter Awaiting requested review from AxelPeter

@frenautvh frenautvh Awaiting requested review from frenautvh

@ganeshkumar1989 ganeshkumar1989 Awaiting requested review from ganeshkumar1989

@Jisay Jisay Awaiting requested review from Jisay

@jleveugle jleveugle Awaiting requested review from jleveugle

@lizardK lizardK Awaiting requested review from lizardK

@radireddy radireddy Awaiting requested review from radireddy

@varun257 varun257 Awaiting requested review from varun257

Assignees
No one assigned
Labels
bug Something isn't working quality check: ok translation required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JeremyDec @antleblanc @FredericEspiau @cbourgois @marie-j