Skip to content
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

applet.control.servo: modify to use port groups #634

Merged

Conversation

isabelburgos
Copy link
Contributor

Part of #599.
This one seems straightforward. I tested that it builds for revC3 with no errors, with and without a --pin-out argument.

@isabelburgos isabelburgos requested a review from whitequark as a code owner July 23, 2024 18:10
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

Looks good other than style differences (which I've pointed out in the comments).

For the servo applet, I think you can test it with glasgow repl and a scope.

@whitequark
Copy link
Member

This PR breaks tests (you need to remove the first get_port_group); actually the error message we show for this is terrible and we should do a better one.

Once you do that, please squash the commits (you can use the "fixup" rebase command to discard their useless messages) and we're good to go!

@isabelburgos isabelburgos force-pushed the isabelburgos/servo-padless branch 2 times, most recently from 7e2f129 to e138345 Compare July 23, 2024 19:47
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

Thanks!

@whitequark
Copy link
Member

Your commit message actually ended up having some unrelated junk in it. Although not a huge issue in this case, we try to keep the git history readable and free of autogenerated noise that can end up in it if you use e.g. GitHub's web interface. Could you remove it please?

@isabelburgos isabelburgos force-pushed the isabelburgos/servo-padless branch from e138345 to a7a43cb Compare July 23, 2024 19:57
@whitequark whitequark added this pull request to the merge queue Jul 23, 2024
@whitequark
Copy link
Member

Thanks for all the updates!

Merged via the queue into GlasgowEmbedded:main with commit b62c254 Jul 23, 2024
20 checks passed
@isabelburgos isabelburgos deleted the isabelburgos/servo-padless branch July 25, 2024 17:58
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.

2 participants