Hi!

thanks, I'm looking at the PR now. I have some reservations to this PR
-- specifically it changes some whitespaces around, but mainly the order
of if...else if statements regarding the units, making somewhat unclear
what actually changed there.

The worst part is -- the PR removes the call to parse_value and since
the function is static, I believe it is unreachable.

I think the PR needs a bit of testing. If the parse_value is somehow not
necessary, it needs to be removed. If it was a mistake, it should be
added. I would also suggest to add examples of the parsed string, and if
possibly the addition of some tests to the parsed values to make sure
the code works (there likely is not a good framework to do it though..).

Best regards,
Ladislav

On Fri, Oct 24, 2025 at 03:06:06PM +0200, ttnp--- via sigrok-devel wrote:
> Hi, 
> 
> harper23 wrote to me about PR #234 at 08.02.2024
> 
> ---------------------------------------------------------------
> Hello sigrok,
> 
> I propose to add the changes to the sigrok repository.
> I don't see any reason why this pull request should be rejected.
> 
> Best regards,
> Helge
> 
> On Fri, 2025-10-24 at 11:20 +0200, Ladislav Laska wrote:
> > Hi All,
> > 
> > Based on the conversation here, it seems everybody wants to keep the
> > project running, so let's make it happen.
> > 
> > I'd like to start a review crunch of the PRs on github, with the idea
> > the project could release a decent version, ideally by the end of this
> > year (we can think of it as a christmas gift to the community). Anybody
> > willing to help?
> > 
> > Please post here if:
> > 
> > 1. There is a PR you've already reviewed and believe it's ready for
> > merge.
> > 
> > 2. There is a PR you want to review & test to help out the project.
> > (this is to so we divide the work properly)
> > 
> > 3. Have special interest in some PR (fixes an issue you're facing, adds
> > support for hardware you have). I'll try to look into those if they are
> > within my skill set (or somebody else can pick them up).
> > 
> > ... or if you want to help in any other way.
> > 
> > 
> > In the first pass, I plan to look at each PR, process what I can test
> > without hardware (there are some general library and compatibility
> > fixes). I'll then check if some of the PRs match hardware I have and can
> > be easily tested. My main interest is to get my LA DSLogic U3Pro16 up
> > and running in mainline sigrok, but I also have some Siglent and Rigol
> > gear at home, work and friends.
> > 
> > Best regards,
> > Ladislav
> > 
> > 
> > _______________________________________________
> > sigrok-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/sigrok-devel
> 
> 
> 
> _______________________________________________
> sigrok-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/sigrok-devel


_______________________________________________
sigrok-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/sigrok-devel

Reply via email to