Skip to content

Spi protos#174

Merged
tyeth merged 3 commits intoapi-v2from
spi-protos
Apr 22, 2026
Merged

Spi protos#174
tyeth merged 3 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.

Comment thread proto/wippersnapper/spi.proto Outdated
Co-authored-by: Copilot <copilot@github.com>
@lorennorman
Copy link
Copy Markdown
Contributor

I got pinged for approval on this in BC but it already has my approval, merge away!

@brentru
Copy link
Copy Markdown
Member

brentru commented Apr 22, 2026

Checks pass, LGTM

@tyeth tyeth merged commit b3411a2 into api-v2 Apr 22, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants