Skip to content

Conversation

@brpowell
Copy link
Contributor

What does this PR do?

Update tooling lwc to use new deploy response

What issues does this PR fix or reference?

@W-7853909@

@brpowell brpowell requested a review from a team as a code owner July 21, 2020 23:03
}
return successes;

if (deployment.diagnostics.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't completely understand this part, aren't diagnostics in this scenario exclusive to a failure ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this is a bit weird. Because of the nature of tooling api we can have some files succeed when upserting pieces of a bundle and some not. I figured if at least some succeeded we'd set the status of the component to "Changed" but there could be a diagnostic for a piece that didn't succeed. I'm not the biggest fan of this but not sure if it should be reported as Failed if some parts did work, what do you think?

Copy link
Contributor

@AnanyaJha AnanyaJha Jul 22, 2020

Choose a reason for hiding this comment

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

If some files fail while others don't, do the files that did succeed get reverted? Or are they still deployed? If there is a partial deployment, then I think it makes sense to keep the status as Changed so the user knows that some pieces were actually deployed.
Sidenote, if we keep the status as Changed, in the vs code side would we still display the diagnostics for the failed portion with the "Failed" progress notification?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because tooling is granular (one transaction per file instead of component) there won't be a revert unlike metadata api.

Sidenote, if we keep the status as Changed, in the vs code side would we still display the diagnostics for the failed portion with the "Failed" progress notification?

Yeah I think it would be something like that. Your comment just made me realize I forgot to actually include the file the diagnostic is related to in the type 🤦 , need to update that

Copy link
Contributor

@lcampos lcampos left a comment

Choose a reason for hiding this comment

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

:shipit:

@brpowell brpowell merged commit dfa05ca into develop Jul 22, 2020
@brpowell brpowell deleted the bp/newApiResponse-lwc branch July 22, 2020 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants