[Bug 902086] Review request: Elasticsearch

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

 



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



--- Comment #90 from Zbigniew Jędrzejewski-Szmek <zbyszek@xxxxxxxxx> ---
(In reply to jiri vanek from comment #88)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #87)
> > (In reply to Zbigniew Jędrzejewski-Szmek from comment #72)
> > > - %description needs to be extended to say what this package does.
> > The new text seems to have been pasted from a website. It's a promotional
> > blurb. It even ends with "Learn more".
> >  
> > > - Consider using %autosetup
> > It would make the spec a bit simpler.
> 
> I still not gert used to it. Please submit patch for me to this issue. Or
> allow me with this oldschool approach.
Basically it's:
- %setup -q -n elasticsearch-%{namedversion}
- %patch0 -p0
- #%%patch1 -p0
+ %autosetup -n elasticsearch-%{namedversion}

Certainly not a requirement, but it makes it easier to keep the list of patches
and their application in sync.

> > > - Can the tests be enabled? If this package is as brittle as it is rumored
> > > to be, tests would be beneficial.
> > I'd like an answer here too.
> 
> Sorry - forgot to it. 
> To enable tests quite a lot of dependencies are needed. IMHO it is not
> possible to make a deadline with them, but on long term they will be pretty
> necessary.
> 
> Si yes, I definitely agre with tests, but considering the date of deadline
> and my free time, I would like to avid it now.
OK.

> > > I'd very much prefer to review the whole thing, including the .service file
> > > if one is to be added.
> > I gather that this will not be run as a service then, but only from the
> > commandline?
> 
> I'm not against service, we (me) is just runnoing out of time a bit. Service
> may be added sooner or later. I hope a bit you (and maybe more CC fromthis
> bug) will become a comaintainer(s) after review  and implement those missing
> features.

The problem is that elasticsearch is a daemon, so having a service file is
actually
required by the Guidelines. Shouldn't be too hard, though java daemons have
some
tricky corners. I'll give it a go.

> > (In reply to gil cattaneo from comment #86)
> > > (In reply to jiri vanek from comment #82)
> > > > After reformanting, this are differences between packed basae64 and included
> > > > one.
> > > > IMHO they should be upstreamed, but it is obviusly not ES way.  Looking to
> > > > them, I would rather kept them bundled.
> > Those changes seem to be mostly cosmetic (encoding fallback, localization,
> > and a check for illegal char).
> 
> 
> Unluckily - mostly. If it would be only the the charset stuf, then I would
> be ok with removal.  But the check for invlaid charactter? I would say we
> can expect problems here.
I don't think so. It'll skip part of an invalid input, instead of erroring out.
You don't expect to feed bad input into it in the normal case, and both
ways to handle invalid input are acceptable, although of course catching it
properly is nicer. But this is the kind of patch that should be acceptable
in the java-base64 package.

> > Your choices are a) unbundling, b) bundling with FESCo exception. 
Sorry once again for replacing FPC with FESCo here.

-- 
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
https://admin.fedoraproject.org/mailman/listinfo/package-review





[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]