Nut upsdev - Apr 2017 - Many open green PRs in NUT, wanna merge?

On Apr 21, 2017, at 12:54 PM, Jim Klimov wrote:
> 
> Hi guys, I can't help noticing that many PRs seem good to go merged in
NUT, so these improvements can either benefit the users instantly, or get
exposed to more real-life testing and prove their worth or find issues to fix ;)
> 
> Are there any objections to going on a merging-spree, and boost the morale
of external contributors along the way? Or, can I go on such a spree using
reasonable judgment? :)
No objections in principle, but let's put this in context. We still have a
number of seemingly simple patches on both the mailing lists and GitHub that
need fairly extensive integration testing, or additional documentation. I
don't think we have done much in the way of triage for the next release. A
few issues have been added to the 2.7.5 milestone, but at this point, we
probably want to pick a few of the PRs below, and defer the rest.

The libusb-1.0 branch is probably the most invasive change:
https://github.com/networkupstools/nut/issues/300
<https://github.com/networkupstools/nut/issues/300> . While it has had
some testing for problem models, I think it is probably the best candidate to
push out for wider testing (as part of 2.7.5, say) - and we want to be careful
that we don't add too many other changes that can't easily be switched
off. We also haven't documented the differences between the libusb versions
outside of the issue log.

Side note: I also would prefer that we not add more merge commits than actual
commits (rebasing still gives the original author credit), but whatever. I think
the more important thing is to not just blindly merge everything that has a
green checkmark, given how our current CI systems do not consider system-level
testing (and will never be able to verify anything more complicated than
spelling/grammar in documentation).

I won't have time to look at the list below in any great detail until maybe
this weekend, but perhaps we can add a milestone for topics like
"documentation for 2.7.5"? I think it helps to have the issues/PRs
sorted into smaller, related lists, especially when some of them partially
supersede others.
> 
> I believe the following should have no major objections:
> * https://github.com/networkupstools/nut/pull/428
<https://github.com/networkupstools/nut/pull/428>
> * https://github.com/networkupstools/nut/pull/427
<https://github.com/networkupstools/nut/pull/427>
> * https://github.com/networkupstools/nut/pull/426
<https://github.com/networkupstools/nut/pull/426>
> * https://github.com/networkupstools/nut/pull/419
<https://github.com/networkupstools/nut/pull/419>
> 
> These may need a review, but seem functional already and can be expanded
upon if needed (by other PRs later), or may just have fallen between the cracks
and forgotten for no good reason:
> * https://github.com/networkupstools/nut/pull/422
<https://github.com/networkupstools/nut/pull/422>
> * https://github.com/networkupstools/nut/pull/423
<https://github.com/networkupstools/nut/pull/423>
> * https://github.com/networkupstools/nut/pull/418
<https://github.com/networkupstools/nut/pull/418> (this one I'd expand
with verification that files are present and not empty before touching but LGTM
otherwise)
> * https://github.com/networkupstools/nut/pull/349
<https://github.com/networkupstools/nut/pull/349>
> * https://github.com/networkupstools/nut/pull/276
<https://github.com/networkupstools/nut/pull/276>
> 
> * This doc update needs a bit of simple merge-conflict resolution but looks
ok otherwise:  https://github.com/networkupstools/nut/pull/401
<https://github.com/networkupstools/nut/pull/401>
> * ...and this one originally had some concerns from @zykh and @clepple -
but the PR structure was revised to break a big commit into many smaller commits
(per-file) and excludes the QX document, so reviewers have a chance to point at
dropping the other commits they really don't like. Or accept merging of the
bunch, if no more strong opinions arise, and perhaps address other concerns in
subsequent PRs ;)
> 
> These had objections not addressed since (but validity of objections may
need revision, just in case - maybe the merges should not really be blocked?):
> * https://github.com/networkupstools/nut/pull/274
<https://github.com/networkupstools/nut/pull/274>
> 
> And this one began to have some feature and size creep, so I'd prefer
to commit the foundations (which should work at least for the initially stated
target of Linux systemd) and base some future point improvements separately...
though maybe can schedule a bit of time to nail the Solaris SMF caveats first,
at least before releasing 2.7.5 (though why should Penguins wait for that other
OS's nuances to be solved? ;) they can look for bugs in their codepath
already ;) ):
> * https://github.com/networkupstools/nut/pull/330
<https://github.com/networkupstools/nut/pull/330>
> 
Some of the feature creep commits were added while I was getting ready to go on
vacation, so I can't say that I have a full understanding of what is going
on there. From a sysadmin's perspective, I wonder if Base64 is the right way
to do this? We should probably expose those conversions in a standalone tool
(Jim, you mentioned something similar on 2017-03-07
<https://github.com/networkupstools/nut/pull/330#issuecomment-284793178> ,
or consider something with more traditional quoting semantics (systemd seems to
do this for device nodes).

Note that I haven't tested this, so there may be some other way to view
driver status in systemd that does not involve viewing Base64-encoded names.
> And finally there is the DMF branch, without a PR yet - but with hundreds
of commits. It did not require maintenance (beside syncing from master) for
several months now, while it works in our project on a daily basis, so I'd
say it is also good to merge. It is waiting for Arno's precious and rare
resource of time to review it for soon-to-be a year...
From the perspective of someone outside Eaton, this is a frustrating branch. It
solves problems that many NUT users don't have, and while the latest few
iterations have been cleaned up tremendously, there is a lot of technical debt
on the configuration/compilation side that seems to be the result of rushing to
add features initially. I suspect that there will be a few issues that crop up
during packaging, and given the time it takes to rebuild and check .debs for the
PPA on other branches, this is not something I look forward to doing in my free
time.

Buildbot is up again, and as of ed4107d2, the DMF branch is failing across the
board in distcheck-light.

Is there an integration test plan for this branch?

-- 
Charles Lepple
clepple at gmail



-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20170426/dc75bb58/attachment.html>