[chirp_devel] [PATCH] [AnyTone 5888UV] Add simple squelch mode & additional file identifier and add extra channel attributes
Dan Smith
Fri Nov 20 13:29:23 PST 2020
> Attached is the patch with my Anytone 5888UV changes folded together.
> This takes the place of those earlier sent:
> [AnyTone 5888UV] Add limited squelch mode & fix file identifier
> [AnyTone 5888UV] Add extra channel attributes
>
> Thanks for the help.
>
> In the future, when I work on multiple issues in the same file(s), is there a better procedure to use so I don’t end up folding them together into one patch when there are changes needed due to feedback from the team? The approach taken this time will combine a bug fix and a feature into a single changeset.
I think there may have been some confusion here, likely me being not super clear and definitely related to me processing a big backlog of patches. So, apologies.
When I asked you to squash things together, I was asking you to do that for the new driver you had proposed, not for these fixes to the existing driver. It's really nice to have separate fixes and features be added in separate patches, which is more of what you had here. Looking at this, *ideally* this would have been three patches:
1. Fix file identifier (really: add a new one you found in the field)
2. Add limited squelch mode (which is kind of a fix, it looks like)
3. Add extra channel attributes (new features).
That way later if we find someone that reports a new problem with, say, the squelch stuff, we can look back and see "ah, here's the full scope of the changes related to squelch stuff, unrelated to extra channel attributes, that's probably where the problem is." Also, this patch will get tagged to all the related bugs, which means people watching a bug about a file ID problem will see a new commit related to it which appears to primarily be about squelch features.
What I was trying to avoid was actually with the other patch (which I still need you to update) is having a commit history that looks like this:
1. Add a new driver that doesn't pass tests
2. Make some more changes
3. Fix the driver so it actually finally works
That is why I wanted you to squash those things. I think the confusion came because you asked if I wanted a patch-to-the-patch in question, which I don't want, because like the new driver example above, it ends up looking like this:
1. Fix a thing
2. No, actually fix a thing
Which we should avoid if we can. So, I said "please send a single patch" because I meant I wanted you to just update and re-send *that* patch. When I said "please squash the multiple patches" I meant for the new driver and its associated revision patches.
In the future, when you're working on a fix or feature and need to make a change to it, just qpush/qpop so it's on top, make a change and qrefresh that change into the patch.
> At some point do I invoke qfinish on my patches or does the series continue to grow indefinitely?
No, when I apply a patch you can just qdelete it from your stack and then 'hg pull' from the main repo when it's applied (as it is now).
This patch also has a lot of style fails related to long lines. To avoid making you go back and revise this again, I'm just going to fix those for you, but in the future, please run the style checks first (tox -estyle).
One last thing, you need to actually put '#' in front of your issue numbers, both because that's how redmine knows to update the issue with a link to the commit, but also because my local commit hook will refuse the commit as a check, if it's not tagged to at least one issue. I've fixed that locally as well, but just FYI for the future.
Thanks!
--Dan
More information about the chirp_devel
mailing list