-
Notifications
You must be signed in to change notification settings - Fork 70
Add 'WIT by Example' section #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add 'WIT by Example' section #287
Conversation
Hey @catamorphism thanks for adding this section -- generally we like to discuss additions like this in issues first before putting up PRs about them and ideally maybe even in a meeting beforehand. I think these changes are great, but would love to socialize the changes more broadly before reviewing/making a move here. @itowlson @kate-goldenring @macovedj thoughts? It would be great to walk through this at the next meeting if this doesn't merge before then. |
Yeah I would consider combining this with the WIT "reference". I like the idea of more examples and more motivation, but the current draft risks two pages to maintain with largely overlapping content. I'm also struggling with two style points here, one as a reader and one as a reviewer. As a reader, the included WIT files are walls of text that don't justify their size. The monotonic clocks example is 6 lines of definitions and 19 lines of doc comments. That's appropriate for a production WIT file, but counterproductive for pedagogy: seeing a doc comment or two is useful, but after that, they clutter the text I'm trying to learn from. And those 6 lines are repetitive - the second type alias and the second function aren't showing anything new. Filesystem types is 95 lines, included as a block mid-page: what does that look like? How does a reader experience that? What does the reader make of being told "oh and this is the simplified version"? I do like that this shows something more realistic than the dear old calculator, but I'd advocate pruning it back to show things in small, digestible chunks, as far as possible, with minimal redundancy except where useful for learning/context. As a reviewer - and I feel a bit silly bringing it up - I'm finding it increasingly hard to follow the raw text/diff view layout. There's places where we use 40-character lines and places where we use full-length lines, and either is fine but in the context of full-length lines the sections of short lines give me an extremely staccato effect. I appreciate that line lengths are never going to be completely even and that nobody wants to spend their life reflowing text as they edit, and I'm not saying we should have rigid guidelines - we shouldn't - but I'd find it way easier to read if the paragraphs flowed more like paragraphs. Sorry, I know that second one is kind of a nit, and that readability is very subjective! Thanks for the hard work on this @catamorphism, I can see that a lot of care and thought has gone into it but I agree with @vados-cosmonic that it would be good to figure out goals and approach before taking this further. |
Concretely with regards to this, we should probably standardize the line wrap we expect. I don't want to be restrictive about it, but at least a posted policy would be good (and if we could make it an automated lint that would be perfect, because people don't get mad at robots for being pedantic). |
I assure you I get mad at formatter lints all the time. More seriously, I'd favour loose policy over pedantic automation. I don't want my PR rejected because a line strayed up to 81 characters or down to 49. I'd rather trust loose consensus and human good taste, even if humans differ! |
I'd be happy to wait until the next meeting to discuss. |
Hey just checking in here but @catamorphism I know we discussed this at the last meeting -- you're not blocked on us here right? IIRC we had a concrete list of things that would be nice to add to get this across the line. |
@vados-cosmonic No, not blocked -- I haven't incorporated the suggestions yet. Thanks for checking! |
I've simplified the examples and removed the doc comments, since the code is explained in the main text. In the SIG meeting we talked about things like including examples of how to use the registry. I thought the smallest unit of work to do as a next step was to include a text-file-based example in a way that's more accessible than the "WIT overview" section, so that's still what this PR does. I have the notes from the SIG meeting and will address those in future PRs. But if you think there's something else that really should be addressed right away (e.g. publishing to the registry), let me know. |
a752b5d
to
12caf68
Compare
component-model/examples/wit-section-examples/clocks/monotonic-clock.wit
Outdated
Show resolved
Hide resolved
Hey @catamorphism I think we're OK to merge this as-is once it's ready! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
I thought it might be helpful to add some WIT examples before launching into the detailed overview.