Skip to content

Spi protos#174

Open
tyeth wants to merge 2 commits intoapi-v2from
spi-protos
Open

Spi protos#174
tyeth wants to merge 2 commits intoapi-v2from
spi-protos

Conversation

@tyeth
Copy link
Copy Markdown
Member

@tyeth tyeth commented Apr 2, 2026

Closes #172

@tyeth tyeth changed the base branch from master to api-v2 April 2, 2026 23:35
@tyeth tyeth marked this pull request as ready for review April 17, 2026 15:19
@tyeth tyeth requested review from brentru and lorennorman April 17, 2026 15:19
Comment thread proto/wippersnapper/docs/display.md Outdated
Comment thread proto/wippersnapper/display.proto Outdated
Comment thread proto/wippersnapper/display.proto Outdated
Comment thread proto/wippersnapper/display.proto Outdated
@tyeth tyeth dismissed brentru’s stale review April 17, 2026 16:22

github doesn't mark requested changes as done despite resolved comments

Copy link
Copy Markdown
Contributor

@lorennorman lorennorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. Loving the overall concept of extracting transport from component type.

My note as usual is about reducing visual noise. I also noticed we prefix a lot of stuff in i2c with Device... that I may apply tidiness pressure to. General rule would be "as simple as possible but no simpler", so I'm happy to be swayed by any reason to keep the verbosity.

To me, ws.spi.Config... reads just beautifully, while Device is redundant and adds noise.

package ws.spi;

/** SPI device configuration — bus selection and pin assignments. */
message DeviceConfig {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just Config?

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