Skip to content

Unhandled return values in TTYPort::open#322

Open
NONnonHere wants to merge 5 commits intoserialport:mainfrom
NONnonHere:main
Open

Unhandled return values in TTYPort::open#322
NONnonHere wants to merge 5 commits intoserialport:mainfrom
NONnonHere:main

Conversation

@NONnonHere
Copy link

@sirhcel Can you check this
Is it solving the TTYPort issue raised.

NONnonHere and others added 3 commits March 5, 2026 22:03
This also includes harmonizing the nomenclature for our OwnedFd::into_raw which
is called into_raw_fd in the standard library.
@sirhcel
Copy link
Contributor

sirhcel commented Mar 6, 2026

Thank you for preparing a PR @NONnonHere! And especially for also spotting that our OwnedFd could be used in TTYPort::pair too!

I streamlined up some spacing and line breaks in the initial commit with rustfmt and did some additional cleanup for use declarations and paths. Could you please add a changelog entry?

Please ignore CI issues related to MSRV (which fail due to dependencies no longer supporting Rust 1.59.0) and cargo-semver-checks (which seems to have an issue with Rust 1.94.0 and works just fine with an older release).

@NONnonHere NONnonHere marked this pull request as ready for review March 7, 2026 05:09
@sirhcel
Copy link
Contributor

sirhcel commented Mar 7, 2026

Thank you for updating the PR @NONnonHere! I found the changelog but there are also things puzzling me:

  • What is behind merging the main branch back into this PR? I don't see the point in doing this for a PR in the first place and so I'm wondering why.
  • I found the changelog in b195a50 but there are also a bunch of changes from de68572 in this commit. What is behind not adding the changelog on top of de68572 and instead duplicating changes?
  • Why does 6f7b028 revert introducing OwnedFd in TTYPort::pair? I praised this a a good catch for improvement beforehand.

The module paths in tty.rs were somewhat inconsistent but this was nothing you touched. It was rather the moment for me to consistently use one or the other variant throughout the file while at it.

After all the stuff I'm wondering about here, I'd like you to ask about some of the ideas and your workflow out of curiosity because I haven't seen such things in the past:

  • Where did you get the idea from to merge back the changes from main into the PR?
  • Do you use a coding agent or LLM support for crafting commits?

@NONnonHere
Copy link
Author

NONnonHere commented Mar 7, 2026

Hey @sirhcel, apologies for the inconvenience regarding the changelog. I pushed an automated update using claude agent as I was heading out and didn't give it the necessary review.

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