Skip to content

Add new section 'Supporting a new SoC' #99

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mbrossard
Copy link

No description provided.

Copy link

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

this is a random passing-by review, i don't have a stake in this but saw it and was positively surprised by it!

one thing i'm wondering: it seems that a lot of HAL activity is now centered around embassy (with the exception of e.g. esp-hal). i'm wondering if this should be explicitly pointed out so that vendors could opt to implement their PACs & HALs directly in embassy rather than having them separate? OTOH embassy is not a WG project, thus this might be the wrong place for this?

@9names
Copy link

9names commented Jun 17, 2025

vendors could opt to implement their PACs & HALs directly in embassy rather than having them separate?

This is not required by embassy, and vendors typically prefer to keep their codebases in their own org/repo so they can control access to them, there's no doubts about ownership, etc.

Also: maybe ask dirbaio before you start suggesting that everyone shove their code in his project's repo?

@rursprung
Copy link

Also: maybe ask dirbaio before you start suggesting that everyone shove their code in his project's repo?

sorry, this wasn't meant as a "hey, everyone should be doing this!" and more as a "is it the idea that this could/should be done?". sorry if that didn't come across like that

CC @Dirbaio

@felipebalbi
Copy link

This looks great to me, thanks for updating.

@BartMassey
Copy link
Member

Sorry to have lost track — where are we with this? The commits are marked "outdated" but I don't know what that means: Github is hard.

I'd be happy to review and merge it if this is the time.

@rursprung
Copy link

The commits are marked "outdated" but I don't know what that means: Github is hard.

@BartMassey: the comments are marked outdated because @mbrossard pushed a new commit on top, i.e. the comments are no longer attached to the newest commit. the PR itself is still marked as draft - @mbrossard: if you believe that the PR is ready for review could you please mark it as such?

@BartMassey
Copy link
Member

@rursprung Ah, thanks. I'm used to folks force-pushing amended updates to PRs, which I don't believe does this?

@rursprung
Copy link

@rursprung Ah, thanks. I'm used to folks force-pushing amended updates to PRs, which I don't believe does this?

it does the same (source: i very regularly force-push amended commits to open PRs)

@BartMassey
Copy link
Member

Thanks!

Suggested flow based on @jamesmunns's guide in Embassy FAQ
@mbrossard mbrossard marked this pull request as ready for review July 16, 2025 02:32
@mbrossard mbrossard requested a review from a team as a code owner July 16, 2025 02:32
@mbrossard
Copy link
Author

Based on input from @diondokter, @jamesmunns, @adamgreig, @therealprof, @BartMassey and others at RustWeek Unconference.

BartMassey
BartMassey previously approved these changes Jul 16, 2025
Copy link
Member

@BartMassey BartMassey left a comment

Choose a reason for hiding this comment

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

This looks really great to me. Thanks for persisting in the process; I know it's been a bit rough.

Highly appreciated.

Copy link

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

minor nits just from glancing over the changes. an in-depth review needs to come from someone involved in the WG and/or the process (i never had to build a PAC or HAL)

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.

5 participants