Skip to content

Conversation

@JayKickliter
Copy link

@JayKickliter JayKickliter commented Oct 21, 2025

This is PR 1 of 2 adding TCP functionality to uefi-rs and is a replacement for #1779. I removed the high-level uefi crate additions and will open a separate PR for them.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

Generally looks very good, thanks for working on this! I left some minor remarks.

@phip1611
Copy link
Member

The typos CI step should be fixed when you add "ANDed" to extend-ignore-identifiers-re in .typos.toml

@JayKickliter JayKickliter force-pushed the jsk/uefi-raw/add-tcpv4 branch 2 times, most recently from 3641ccb to 95e20c4 Compare October 22, 2025 17:21
@phip1611
Copy link
Member

phip1611 commented Nov 9, 2025

Ping @JayKickliter , what's the status here?

@JayKickliter
Copy link
Author

JayKickliter commented Nov 11, 2025

Ping @JayKickliter , what's the status here?

It should have all review comments addressed now.

@JayKickliter JayKickliter force-pushed the jsk/uefi-raw/add-tcpv4 branch from ef0785f to 2f4310b Compare November 11, 2025 18:04
@phip1611
Copy link
Member

I think unfortunately, you've missed #1797 (comment) and #1797 (comment)

@JayKickliter JayKickliter force-pushed the jsk/uefi-raw/add-tcpv4 branch from 2f4310b to 0c0612b Compare November 12, 2025 17:39
@phip1611
Copy link
Member

phip1611 commented Nov 12, 2025

almost there, thanks! Just one final thing from my side: #1797 (comment)

@JayKickliter JayKickliter force-pushed the jsk/uefi-raw/add-tcpv4 branch from 0c0612b to 074ff74 Compare November 12, 2025 18:24
@JayKickliter JayKickliter changed the title uefi-raw: add Tcpv4 protocol type definitions uefi-raw: add Tcp4 protocol type definitions Nov 12, 2025
@phip1611
Copy link
Member

LGTM! how about @nicholasbishop ?

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.

3 participants