Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign uplibstd::sys take 2 #30777
Conversation
rust-highfive
assigned
aturon
Jan 8, 2016
This comment has been minimized.
This comment has been minimized.
|
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
|
Thanks for the PR! This is such a large change, however, that this falls under the "needs an RFC" category before sending a PR with the changes. If you need any help writing an RFC, however, please just let me know! |
alexcrichton
closed this
Jan 11, 2016
This comment has been minimized.
This comment has been minimized.
|
Internal implementation details require a RFC? I can certainly split it up into smaller chunks for review, or have it land in small manageable parts, but I fail to see why an RFC is necessary or what it would say? It's simply a refactor in order to make porting to new platforms easier. A lot of moving code around but no actual changes. |
This comment has been minimized.
This comment has been minimized.
|
This is a massive change to libstd, even if organizationally. Lots of people work on libstd and it's architected pretty deliberately today, and in general we prefer to gather feedback about changes like this not on PRs but in other forums like discuss or RFCs |
This comment has been minimized.
This comment has been minimized.
|
Well, The biggest issue is how large the changes are, which can cause conflicts with existing PRs and working branches people may currently have. I can try and whip up an RFC if you insist, but I can just see that stagnating for months due to lack of discussion/interest/etc. and I'd really like to see it move forward if possible. |
This comment has been minimized.
This comment has been minimized.
|
Sorry I don't have a lot of time to dig into this much right now, but I know that I at least would benefit from a refresher explaining what's going on here (as it's been awhile since the original PR and the internals thread were opened). I'd personally like to be reminded of:
That fits the skeleton of an RFC at least, and it may be the case that the @rust-lang/libs team ends up deducing that an RFC wasn't worth it after all and settle on the RFC quickly. (others can feel free to chime in here as well, too) |
This comment has been minimized.
This comment has been minimized.
|
I agree with @alexcrichton that this kind of substantial change to implementation details needs to go through the RFC process. We need to hash out the tradeoffs involved before landing it. @arcnmx I know you were worried about not getting enough team attention on the discuss thread, but the libs team tries pretty hard to stay on top of RFCs. Also, writing up a formal RFC is a good way to drive articulation of the pros and cons of a change. |
This comment has been minimized.
This comment has been minimized.
|
I like the goal of centralizing the plattform-specific parts of std into a porting layer. From a quick browse I think I like where this is going. It is a massive patch to digest though and I agree it needs a lot of consideration, and an RFC is probably prudent. |
arcnmx commentedJan 8, 2016
I'm just going to reopen this discussion here because I've been sitting on this for ages and dunno what to do with it. If there's any sort of agreement I can rebase and update it onto master. The end goal here is to move all platform-specific code into
libstd::sys, and remove as many#[cfg()]directives from the rest oflibstdas I can. This makeslibstdcleaner and easier to port to other platforms.See the internals thread for previous discussions. It's massive, I'm sorry... Would it work better if it were split up somehow? I can do it in chunks as long as the final design is agreed upon as sufficient.