Uh oh!
There was an error while loading. Please reload this page.
Stabilize Seek::stream_position (feature seek_convenience) - #70904
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
LukasKalbertodt
commented
Apr 9, 2020
FYI: a new discussion potentially arguing against stabilization just emerged in the tracking issue. |
SimonSapin
commented
Apr 9, 2020
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:
|
LukasKalbertodt
commented
Apr 9, 2020
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. |
Dylan-DPC-zz
commented
May 15, 2020
r? @dtolnay |
dtolnay
commented
May 28, 2020
@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. |
sfackler
commented
May 28, 2020
@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. |
m-ou-se
commented
Dec 30, 2020
@Amanieu@dtolnay I copied your checkmarks from the previous FCP proposal on this PR: #70904 (comment) |
Dylan-DPC-zz
commented
Jan 17, 2021
@KodrAus@sfackler@withoutboats this is waiting on your fcp vote. Thanks |
rfcbot
commented
Jan 17, 2021
🔔 This is now entering its final comment period, as per the review above. 🔔 |
pthariensflame
commented
Jan 23, 2021
|
55689e2 to
8a18fb0Comparerfcbot
commented
Jan 27, 2021
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. |
m-ou-se
commented
Jan 27, 2021
@bors r+ rollup |
bors
commented
Jan 27, 2021
📌 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.