[Bug 1507103] Review Request: kronosnet - Multipoint-to-Multipoint network abstraction layer for High Availability applications

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1507103



--- Comment #64 from Fabio Massimo Di Nitto <fdinitto@xxxxxxxxxx> ---
(In reply to Jan Pokorný from comment #63)
> > Because it´s impossible to follow those jumps
> 
> Really?  Enumerating separate points was to prevent any confusion.

Yes really.

> 
> > you do around between comments and we have no idea what you are
> > talking about related to A.  What is A?
> 
> For whoever is "we", A. was defined in [comment 55]...


"we" are all the people currently involved in this review to move this package
forward.

> 
> > How about adding some context or spec file snippets to make
> > it clear?
> 
> ...see [attachment 1402543 [details]], lines 25,26, 28-30, 458-460 in the
> original.  Hopefully it clarifies it fully.
> 

was it that hard to reference the original lines?

The debuginfo generation is default: on in fedora. Those statements have no
effect on fedora unless explicitly overridden. Those are coming from upstream
spec file that requires tuning to build debuginfo on Opensuse. They create no
harm and have no effect on Fedora. The end result is the same.

Something you missed in the process is that as the review goes, we are merging
all those bits back into upstream spec file so that it can build properly both
for suse and fedora / rhel / centos and reduce the need to maintain multiple
spec files around (after all as you somehow agreed below in another context we
want to kill redundancy).

Given that those are more useful on opensuse we can add a:
%if 0%{?suse_version}
somewhere later on to make them even more transparent for fedora.

Let´s move on.

> 
> >>>> [re E.]
> >>> 
> >>> You haven´t answered either of my questions.
> >> 
> >> My answer was: let's just do it.
> >
> > My questions are still unanswered. I asked if it is breaking
> > policies or not.
> 
> Let me explain that the review is not a black-or-white
> mechanical process, which appears to be your current view.
> 
> It's meant to combine experience and common sense to find
> near-optimal solutions, individually.

Oh finally you start talking like a real mentor that a good reviewer should be.

> 
> In this case, the undisclosed reason behind my suggestion is the common
> sense one: to eliminate redundancy.  To keep Fedora scalable regarding
> mirrors, minimal installations, etc., it's really good to consider
> whether the identical files need to be carried over twice per each
> architecture when not necessary.  Common sense hence says, it's enough
> -- thanks to intra-srpm dependencies -- to carry just a single copy
> per arch.  Packaging guidelines give the green light to this
> "optimization".  I see I should have been more patient to provide
> this explanation first-hand, sorry about that.
> 
> Would you be willing to make this change, i.e., to drop license files
> from libknet1-devel subpackage, now?

Yes of course, now that you provided a good rationale on your request, I have
no objections to make changes.

> 
> * * *
> 
> > hmmm does fedora-review -rn rebuilds the package on the local system
> > (aka f29?) or does it build in a chroot for f27 and test the end
> > result.f27 binaries?
> 
> It built the package detached from host, using mock (systemd-nspawn
> containers), which in turn obtained buildroot with
> > /usr/bin/dnf builddep --installroot \
> > /var/lib/mock/fedora-rawhide-x86_64/root/ --releasever 28 \
> > /var/lib/mock/fedora-rawhide-x86_64/root//builddir/build/SRPMS/kronosnet-1.1-3.fc28.src.rpm
> 
> with toolchain:
> > gcc-8.0.1-0.14.fc28.x86_6
> > binutils-2.29.1-19.fc28.x86_64
> > glibc-2.27-3.fc28.x86_64
> 
> Then
> 
> > So I prefer to understand if the upstream check is bogus on fedora or
> > if the test environment (despite being just a minor warning) is being
> > tricked to think it´s a problem.
> 
> in the respective logs, I can see:
> > checking for library containing ceil... -lm
> 
> It corresponds to this observation:
> $ readelf -s /usr/lib64/libc.so.6  | grep ceil || echo none
> > none
> 
> So the warning may be false positive, after all.

Ok, that makes sense.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux