Stabilize Seek::stream_position (feature seek_convenience)#70904
Stabilize Seek::stream_position (feature seek_convenience)#70904bors merged 1 commit intorust-lang:masterfrom
Seek::stream_position (feature seek_convenience)#70904Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
FYI: a new discussion potentially arguing against stabilization just emerged in the tracking issue. |
|
Highfive is configured in https://github.com/rust-lang/highfive/blob/master/highfive/configs/rust-lang/rust.json#L6, and this list (in particular my absence from it ;)) is deliberate. r? @kennytm In general though the right time for a stabilization is after FCP has happened, because of cases like this:
|
Oh, oops. I simply checked https://www.rust-lang.org/governance/teams/library to see if kennytm is in the libs team, didn't see them listed there and assumed this was a highfive bug. Sorry!
I assumed that FCPs mostly happen on the PR and not the tracking issue. At least I think that that's mostly what I saw so far. I even considered asking for FCP on the tracking issues instead of creating a PR but decided against it because I thought FCP on PR is the "proper" way to do it. If you want, I can close this PR until FCP is complete. |
|
r? @dtolnay |
|
@rust-lang/libs:
@rfcbot fcp merge |
|
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns: Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
@rfcbot concern non-atomic-tell stream_position seems totally reasonable to me (maybe modulo a rename), but I'm not really a huge fan of stream_position being implemented by 3 seeks. It's unfortunately non-atomic and (almost?) every seekable construct of finite length should have a more "proper" method to return its length. |
|
Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
@Amanieu @dtolnay I copied your checkmarks from the previous FCP proposal on this PR: #70904 (comment) |
|
@KodrAus @sfackler @withoutboats this is waiting on your fcp vote. Thanks |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
|
55689e2 to
8a18fb0
Compare
|
The final comment period, with a disposition to merge, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC will be merged soon. |
|
@bors r+ rollup |
|
📌 Commit 8a18fb0 has been approved by |
Tracking issue: #59359
Unresolved questions from tracking issue:
stream_lenforFile?" → we can do that in the future, this does not block stabilization.lenandposition?" → as noted in the tracking issue, both of these shorter names have problems (lenis usually a cheap getter,positionclashes withCursor). I do think the current names are perfectly fine.stream_positiontotell?" → as mentioned in the comment bringing this up,stream_positionis more descriptive. I don't thinktellwould be a good name.What remains to decide, is whether or not adding these methods is worth it.