Rahix
Fix doctests in `attiny-hal`
This PR fixes all existing doctests in attiny-hal and adds cargo test --doc to CI for attiny-hal. Stacked on https://github.com/Rahix/avr-hal/pull/605.
On Sun, Dec 29, 2024 at 05:08:53PM -0800, Iris Artin wrote:
@innermatrix pushed 1 commit.
928daa8aa8a3028599c40d5503a991b46fa1f3b7 Fixed attiny doctests
Hey, #617 has also "Fixed attiny doctests"
On Sun, Dec 29, 2024 at 05:08:53PM -0800, Iris Artin wrote:
@innermatrix pushed 1 commit. 928daa8 Fixed attiny doctests
Hey, #617 has also "Fixed attiny doctests"
Yeah I have several PRs that are all against main but they are stacked on top of each other so the later ones include all the commits from the previous ones. If you'd like me to update them so that each is against the topic branch of the previous one, I can do that.
(I wish GH had better support for stacked PRs, but it doesn't, so here we are; I am happy to use whatever workflow works best for you.)
On Mon, Dec 30, 2024 at 06:06:57AM -0800, Iris Artin wrote:
On Mon Dec 30 2024 Geert Stappers wrote:
On Sun, Dec 29, 2024 at 05:08:53PM -0800, Iris Artin through github:
@innermatrix pushed 1 commit. Fixed attiny doctests
Hey, #617 has also "Fixed attiny doctests"
Yeah I have several PRs that are all against
mainbut they are stacked on top of each other so the later ones include all the commits from the previous ones. If you'd like me to update them so that each is against the topic branch of the previous one, I can do that.
Best answer to "If you'd like" is asking "How would I like it myself".
I myself would NOT like to be flooded with merge requests. I would be annoyed when I had to review the same commit at several places.
So yes, it is a good thing to present commits with a mindset of "My commits are a pleasure to review".
Groeten Geert Stappers More bystander as project member. (Rahix is the project lead.)
Silence is hard to parse
On Mon, Dec 30, 2024 at 08:05:42AM -0800, Geert Stappers wrote:
On Mon, Dec 30, 2024 at 06:06:57AM -0800, Iris Artin wrote:
On Mon Dec 30 2024 Geert Stappers wrote:
Hey, #617 has also "Fixed attiny doctests"
Yeah I have several PRs that are all against
mainbut they are stacked on top of each other so the later ones include all the commits from the previous ones. If you'd like me to update them so that each is against the topic branch of the previous one, I can do that.Best answer to "If you'd like" is asking "How would I like it myself".
I myself would NOT like to be flooded with merge requests. I would be annoyed when I had to review the same commit at several places.
So yes, it is a good thing to present commits with a mindset of "My commits are a pleasure to review".
Groeten Geert Stappers More bystander as project member. (Rahix is the project lead.)
What I remember, there were messages like:
@innermate: What if refactor some parts? @stappersg: Come back with something to review.
What I can't remember to have seen is any message from Rahix about the refactoring proposals.
@innermate: Your "giving source code back" attempt, might not be what you expect, might not be what you hope for. But hey, you are allowed to keep your fork.
Groeten Geert Stappers
Silence is hard to parse
tldr: Thank you and @Rahix for all your great work; I think my contributions would be valuable to the community, so I would love to align with you and @Rahix — in whatever way you find supportive — on my proposed direction. I think a good place to start is for you two to give feedback in #605 about my proposal to have one module to describe each MCU (for example, attiny85.rs). Everything else I think we can figure out downstream.
A longer version and responses to some of your specific points:
Best answer to "If you'd like" is asking "How would I like it myself".
Thank you for saying that — I very much agree with that as an organizing principle. On this particular question, I have seen two different approaches, and I have used both successfully, which is why I picked one and am letting you guide me as to whether you prefer the other. (The two approaches being: 1. every stacked PR is against main and reviewers know to select a subset of commits when reviewing or 2. every stacked PR is against another PR's topic branch and maintainers know in which order they need to be merged.)
Unfortunately, approach 2 (which is kinder to reviewers) is not supported by GH for cross-fork PRs. The only way I could make that work is if I was able to create PRs directly in the main repo.
So for now I am just going to hold off to avoid creating more burden for you; please see below on what I think is the most valuable to review right now. And I do apologize if I overwhelmed you with too many PRs and not enough information about where to focus your limited reviewing energy.
What I remember, there were messages like: @innermatrix: What if refactor some parts? @stappersg: Come back with something to review. What I can't remember to have seen is any message from Rahix about the refactoring proposals.
You remember correctly!
I think there are two main issues we are running into here. One is that it was not clear to me whether you or @Rahix were the project lead at this time, so I wasn't sure how much involvement from @Rahix to expect; thank you for clarifying that.
The other issue is that the changes I am proposing are fairly foundational, and as a result, it's not easy to present a small thing for review. (Breaking it up into smaller pieces actually ends up being more work because of the way that the different parts currently interact with each other. See below for more on this, though.
But hey, you are allowed to keep your fork.
If that's how it turns out, that's okay, but I would much prefer to create something that we can agree to merge back in.
As you saw, I started some additional work that improves documentation and reliability (by adding compile-only doctests for all MCUs to CI). I also have the beginnings of support for ATtiny 0-series and 1-series MCUs that I plan to submit in a future PR. Finally, I have been thinking about what it would take to run unit tests against real AVR hardware, and I plan to bring that up for discussion when I have some coherent thoughts.
I think that those contributions would likely be valuable to the community, and so I would much prefer to integrate them with the great work you are already doing, instead of creating a long-term divergence. I would be happy to work with you and @Rahix on figuring out what form those contributions should take, just let me know what works best for you.
My goal here is to give back to the community in whatever form is the most supportive of you two. What I ask in return is guidance about the direction you want to take with this, and ongoing open communication.
To make that easier, let me ask a specific question:
When you have the time, could you or @Rahix please look at my proposed structural changes in attiny-hal (which submitted in https://github.com/Rahix/avr-hal/pull/605) and tell me whether you think this is a good direction. The main structural change I am proposing in that PR is to refactor the crate to have a separate module for each MCU, which acts as a description of that specific device (as opposed to the current code, which is organized by peripheral). You can see the big picture structure in https://github.com/innermatrix/avr-hal/tree/attiny-hal-additive-features/mcu/attiny-hal/src and the per-MCU structure in any of the files in there (such as https://github.com/innermatrix/avr-hal/tree/attiny-hal-additive-features/mcu/attiny-hal/src/attiny85.rs)
We can (and should) have a discussion later about lesser questions (naming, style, implementation details, etc), but the main thing that I am proposing to do, and that everything else I would like to contribute is built on, is that structural change: one module to describe each MCU. So that's the main question that I would like to align on early.
Once again, thank you!