Skip to content

Conversation

@eliasnaur
Copy link
Contributor

No description provided.

@eliasnaur
Copy link
Contributor Author

The test failures don't seem relevant, but I could easily be mistaken.

@deadprogram
Copy link
Member

The fix for nix is in... or at least for the CI. Please rebase against latest dev.

@dgryski
Copy link
Member

dgryski commented Oct 17, 2024

LGTM but I really want @aykevl's take on this.

return false
}
} else {
case !use.IsALoadInst().IsNil():
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a comment to this block so it doesn't get lost next to the default label?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

Yes this looks reasonable (and I agree a comment before default: would look slightly better).

Copy link
Member

@dgryski dgryski left a comment

Choose a reason for hiding this comment

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

LGTM

@aykevl aykevl merged commit 539cc5c into tinygo-org:dev Oct 18, 2024
17 checks passed
@eliasnaur eliasnaur deleted the bytestrings branch April 12, 2025 06:15
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