Hi Carsten,

Thanks for taking a look. I've added my changes in 1755727 but have a
couple of questions, if you could help.
> why is the interface HtmlClientService empty? I guess this makes this
service unusable and there is no way to create an HtmlClient.
Sorry about that. Funny thing is I tested that with the felix console
script and I think it worked, thanks to polymorphism. But I fixed it now.

> There is an   AbstractHtmlClientImpl but just one implementation. Can't
this be merged into a single class?
AbstractHtmlClientImpl takes away all the http stuff and leaves an abstract
method that would allow it to easily write a new HtmlClient that returns a
different type of Document. Might not have one now, but I'm using that in
an external project and would allow anyone to easily achieve that. Is it a
big problem? I personally would like to leave it.

> Although org.apache.sling.hapi.client.forms is exported, it's only use is
in an implementation. So why not make this private?
You're right again :) What I'm having trouble with the meanings, due to my
lack of expertise in OSGi and mixing osgi concepts and bnd maven plugin
isntructions. Do you mean not exporting it (i.e. forbidden), or making it
private via the maven bundle plugin? I added "<Private-Package>
org.apache.sling.hapi.client.forms</Private-Package>" to the bundle plugin
and the maniefst file doesn't seem to change :/ Anything I'm doing wrong?
How should a private package show? Any way I can make it work?

> There is no javadoc for Document and it's hard to figure out how to use
it.
Sorry, added javadoc.

I will drop this release, sort out these issues and continue from there.
Thanks a lot.
- Andrei

On Wed, Aug 10, 2016 at 7:32 AM Carsten Ziegeler <[email protected]>
wrote:

> Hi,
>
> I had a brief look, and have some comments - sorry that I couldn't look
> earlier:
>
> - why is the interface HtmlClientService empty? I guess this makes this
> service unusable and there is no way to create an HtmlClient.
> - there is an   AbstractHtmlClientImpl but just one implementation. Can't
> this be merged into a single class?
> - although org.apache.sling.hapi.client.forms is exported, it's only
> use is in an implementation. So why not make this private?
> - There is no javadoc for Document and it's hard to figure out how to
> use it.
>
> I personally think we should fix these things first.
>
> Regards
> Carsten
>
> > Hi,
> > We solved 3 issues for this initial
> > release:
> https://issues.apache.org/jira/browse/SLING/fixforversion/12337959
> > Staging repository:
> https://repository.apache.org/content/repositories/orgapachesling-1498/
> > You can use this UNIX script to download the release and verify the
> > signatures:
> http://svn.apache.org/repos/asf/sling/trunk/check_staged_release.sh
> > Usage:sh check_staged_release.sh 1498 /tmp/sling-staging
> > Please vote to approve this release:
> >
> >   [ ] +1 Approve the release
> >   [ ]  0 Don't care
> >   [ ] -1 Don't release, because ...
> > This majority vote is open for at least 72 hours.
> >
>
>
>
>
> --
> Carsten Ziegeler
> Adobe Research Switzerland
> [email protected]
>
>

Reply via email to