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: disallow undefined xml #757

Merged
merged 6 commits into from
Sep 18, 2023
Merged

fix: disallow undefined xml #757

merged 6 commits into from
Sep 18, 2023

Conversation

mshanemc
Copy link
Contributor

@mshanemc mshanemc commented Sep 18, 2023

What does this PR do?

existing code was allowing xml: undefined to match the SourceComponent type, but it doesn't really. Then other code that expected a SC would throw unexpectedly

What issues does this PR fix or reference?

found during QA of forcedotcom/source-deploy-retrieve#1093

@W-14138121@

@mshanemc mshanemc added the bug Something isn't working label Sep 18, 2023
@git2gus
Copy link

git2gus bot commented Sep 18, 2023

This issue has been linked to a new work item: W-14138121

Copy link
Contributor

@shetzel shetzel left a comment

Choose a reason for hiding this comment

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

Worth adding a unit test?

@cristiand391
Copy link
Member

QA notes:

before:

➜  dreamhouse-lwc git:(main) ✗ cat .forceignore
# First, ignore everything
*
➜  dreamhouse-lwc git:(main) ✗ sf project deploy preview -o dreamhouse
Error (1): The "path" argument must be of type string. Received undefined

with this fix:

➜  dreamhouse-lwc git:(main) ✗ cat .forceignore
# First, ignore everything
*
➜  dreamhouse-lwc git:(main) ✗ sf project deploy preview -o dreamhouse

No conflicts found.

No files will be deleted.

No files will be deployed.

No files were ignored. Update your .forceignore file if you want to ignore certain files.

@cristiand391 cristiand391 merged commit 42d9409 into main Sep 18, 2023
35 checks passed
@cristiand391 cristiand391 deleted the sm/disallow-undefined-xml branch September 18, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants