[Bug 902086] Review request: Elasticsearch

[bugzilla at redhat.com]

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



--- Comment #88 from jiri vanek <jvanek at redhat.com> ---
(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.

> 
> > - 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.

> 
> > 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.

> 
> (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.

> 
> Your choices are a) unbundling, b) bundling with FESCo exception. I'd
> strongly advise against asking for a bundling exception, especially that in
> this case it simply does not seem worth it, and might cause this package to
> miss Alpha Freeze, and is likely to be denied. I'd suggest instead opening a
> bug against java-base64 to look at those changes and incorporate / upstream
> them if they look OK. ElasticSearch will almost certainly work fine without
> them, so the package can be approved without waiting for them.

Yes, C is right.

Upstream changes and then make ES to use this nev version.

I would like to go with B now, and in lifetime move to C.

Actually - Gil - is it acceptable for you to include patch in RPMs? ( I guess
not, and also I'm not advising it, just asking)

-- 
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