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

Modify the fix-2805 transcript to use return type #5586

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

sellout
Copy link
Contributor

@sellout sellout commented Feb 20, 2025

Overview

It previously used printLine to print the result from the main
function, but transcripts don’t capture stdout, so this changes it to
return the result instead, so it’s reflected in the output.

This is a follow-up to #5549, based on this comment.

It previously used `printLine` to print the result from the `main`
function, but transcripts don’t capture stdout, so this changes it to
return the result instead, so it’s reflected in the output.

This is a follow-up to unisonweb#5549, based on [this comment](unisonweb#5549 (comment)).
@sellout sellout requested a review from aryairani February 20, 2025 21:56
Copy link
Contributor

@aryairani aryairani left a comment

Choose a reason for hiding this comment

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

Looks good; not sure why the transcript crashed in CI?

@aryairani
Copy link
Contributor

Looks good; not sure why the transcript crashed in CI?

@ceedubs Could this be a regression in #5584 / #5585? Seems unlikely, but...

@sellout
Copy link
Contributor Author

sellout commented Feb 20, 2025

@ceedubs Could this be a regression in #5584 / #5585? Seems unlikely, but...

Yeah, #5585 is failing on trunk, with a hash mismatch in transcripts. Probably the hash changed with some other commit, and merge wasn’t blocked because there wasn’t actually a conflict.

@aryairani aryairani merged commit c89b687 into unisonweb:trunk Feb 20, 2025
14 of 17 checks passed
@aryairani
Copy link
Contributor

Gotcha thanks.

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