On Tue, 2020-06-09 at 21:39 +0200, Arno Morbach wrote:
>
> I made a pull request for the SPDIF decoder in January (Pull request
> #26). Until now nothing happened. There is no response and I don't
> know if I made a mistake and the pull request is not really visible
> on your side. Or will the pull requests sometime be evaluated in a
> certain time slot?
Not speaking for the project, just stating my personal view as
one of the (more active) users. So take it with a grain of salt.
Yes the github pull requests are only visible to those who are
subscribed to that platform, or those who actively scan the list
of requests. So I usually never notice them, which may or may not
be what others experience as well. Providing feedback seems to be
limited in simlar ways, can't do that without a github account
(which I don't have, and don't plan to get).
Sending a ping is fine, which is what you did. There is the
alternative of comparing your submission to what else is there so
far. Having cloned the repo(s), you have access to their history.
Can you spot the differences? A lot can be learned from observing
what has worked for others in the past. Another alternative to
github and bugzilla (which are rather "static" and more "for the
record" and for archiving purposes) and the mailing list (which
"is slow" and is severely limited in terms of fast interaction or
having a fluid conversation) would be IRC where most developers
hang out.
Your submission appears to improve the decoder's implementation,
but suffers from some style issues. That's why the PR cannot be
used in its current form, more massaging is required before
integration with the project. These are the most important
changes that come to mind:
- Separate the different aspects that you address, create a set
of commits which address individual parts of the problem (make
samplerate optional, improve robustness of width detection,
improve the synchronization phase and add diagnostics, etc).
This simplifies review. Smaller changes which address a single
issue are easier to reason about than a huge commit which
changes a lot of things at the same time. You'd only do huge
commits for massive reworks that are hard to separate after the
fact of actually rewriting most of what was there before. Not
sure if that condition applies to your recent fixes.
- Rephrase the commit messages, take the position of those who
read them later (during research, or maintenance), and consider
how the messages get perceived (_after_ the change was included
in the code base). "Add files via upload" is not a good
description of what happened to the source, and how it affects
its operation. See existing history for examples. Consider
"zoom levels" of commit messages, one-liners for fast
navigation, and more details when "zooming in". Make sure to
mention the bug report's number in commit messages, so that
others can see that more information or data is available.
- Address the style issues. Consider trimming the line length to
increase readability, fix up whitespace (both leading and
trailing) to unbreak other people's navigation in the source,
adjust comment style for consistency with other parts of the
project. Other style issues were there before, but are worth
addressing when the implementation gets more complex (eliminate
magic numbers, use more Python idioms or drop idims that are
not preferred any longer, drop redundancy in variables, etc).
- Did the decoder implementation change in incompatible ways to
the previous version? Which might be OK if there were severe
limitations or omissions in the previous version. But which
also affects the test suite. Does the new version still pass
the test suite, _and_ pass the new conditions which are shown
by your additional captures? Did you have to adjust existing
test cases, and add new ones? And did the adjustment of the
existing expectations look plausible to you? See the
sigrok-test repo, and the pdtest utility which executes the
decoder tests.
When in doubt about a project's style or ways of submission,
check documents in the root directory of the source that you
cloned. In the sigrok case the HACKING and README files could be
of interest to you.
If you don't feel comfortable with this rework for improved
consistency with the project's style, I may look into that (may
take a few weeks though). In that case: You spent time and effort
and contributed an improvement to the project, which I'd want to
thank you for. Your commits use a noreply address, which is
unfortunate (you shall be recognized as one of those who spent
work on the feature, and future users may want to contact you
since you are the author of or a contributor to a significant
feature). Can I use the mail address that you used here in the
mailing list, for proper attribution of your work?
virtually yours
Gerhard Sittig
--
If you don't understand or are scared by any of the above
ask your parents or an adult to help you.
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
