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(merge-from-scope), do not throw an error about missing artifacts when merging main to a lane #8557

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

davidfirst
Copy link
Member

@davidfirst davidfirst commented Feb 21, 2024

Previously, collectVersionsObjects had a bug, ignoreMissingLocalArtifacts didn't do anything, even when it was false, it was still ignoring local artifacts because locallyChangedHashes.includes(versionObject.hash.toString()) always returned false. The versionObject.hash.toString() printed the hash method instead of the hash content. It's fixed to be versionObject.hash().toString() to make it work.
Because of the that bug, #7119 was adding new flag ignoreMissingExternalArtifacts unnecessarily. It assumed that an error wasn't thrown because it was external which wasn't correct. In this PR this flag has removed.
In addition to this, when merging from scope and merging main to a lane, we can skip the import of the artifacts because once the user asks for the artifacts we retrieve it from main if not found in the lane.

One last thing, because until now ignoreMissingLocalArtifacts practically did nothing, I'm afraid that after this fix it'll throw errors unnecessarily, so I made the default to not throw when missing. We didn't get any complain about missing artifacts in the remote scopes, so it should be fine.
This flag was renamed to throwForMissingLocalArtifacts for clarity. The only place we throw is when merging-from-scope and the source is a lane, not main.

@davidfirst davidfirst enabled auto-merge (squash) February 21, 2024 16:58
@davidfirst davidfirst disabled auto-merge February 22, 2024 14:32
@davidfirst davidfirst merged commit 6ed5015 into master Feb 22, 2024
11 checks passed
@davidfirst davidfirst deleted the missing-artifacts-err branch February 22, 2024 14:32
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.

2 participants