-
Notifications
You must be signed in to change notification settings - Fork 284
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
Editing asset cache output when using x-script #1541
Editing asset cache output when using x-script #1541
Conversation
return urls[0]; | ||
} | ||
|
||
errors.push_back(std::move(maybe_success).error()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is being fairly careful to put the errors in errors
rather than emitting them here directly; can you explain why this case is different than the others?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this error occurs when there is a sha mismatch. I'm printing rather than putting in errors so that the order of the failure messages makes sense and is consistent with the download failure messages below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I would really like to see at least a comment here explaining what the criteria for 'errors' vs being printed directly is, since changing which is which on and ad-hoc basis like this seems likely to create as many bugs as solved every time it is touched.
However, this function is already awful about that so not going to block over this.
See, microsoft/vcpkg#39901 (comment)