Skip to content

Conversation

@sunshowers
Copy link
Contributor

This is an extremely large PR, but a generally quite mechanical one. I had Claude Code both write most of the PR and review it for conformance with the RFD -- all tests appear to pass, and I also manually spot-checked the versions crate to ensure it generally made sense.

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
@sunshowers
Copy link
Contributor Author

@david-crespo one of the things we determined in RFD 619 is that we organize types by semantic area rather than params/views/shared. This makes sense for internal APIs but would love your thoughts on this when applied to the external API (I can put things back in params/views/shared if desired, but wanted your thoughts on the complete migration).

@david-crespo
Copy link
Contributor

I don't think it's a big deal either way. I'll think about it. What I like about importing views and referring to views::Disk etc. is that when you have a bunch of things all called Disk — model, view, params — it’s nice to easily tell which one you’re dealing with. On the other hand there are various other ways to tell. In this case the variable is called params, and the Create in the name also makes it clear, though that's not always the case. The type hover doesn't really tell you:

Screenshot 2025-12-23 at 9 00 04 PM

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