David M Johnson wrote:
Comments below...
On Mar 28, 2006, at 3:35 PM, Allen Gilliland wrote:
David M Johnson wrote:
On Mar 28, 2006, at 1:08 PM, Allen Gilliland wrote:
now that i think about it, maybe it is not a good idea to squeeze
this in at such a late time in the release process.
I see that last point, but these changes are isolated to two
Servlets, don't impact any existing features and are disabled by
default.
yes, but the moment they go out in a release we are required to
support them forever, so we had better be sure about it.
I totally disagree with that. Nothing obligates us to support this
forever. And specifically for the Atom protocol, people are going to
target Atom protocol 1.0, not draft 8 (which is what we now
support).
The folks who target draft 8 are the folks trying to help IETF
get the
protocol right (i.e. the folks in the working group).
I would not propose moving out of the sandbox if I didn't think Atom
protocol was stable, very near complete and ready for serious
interop
testing.
I am not saying that the protocol or the code is not stable, but I do
think that once we release something in a fully sanctioned Roller
release then we are committing to support for it.
By support I mean that we are suggesting that we won't purposely
break
the implementation in future releases and we will maintain some
level of
backwards support for anyone who starts using it. We can
certainly add
this to the application with a warning stating that, "This feature is
experimental and unsupported. Any use of this feature is subject to
change in the future. If this breaks your site then it's not our
fault."
I can live with that as long as it is clearly understood by anyone
who
tries to use it that we may make any changes to the code in the
future
without regard for backwards compatibility or legacy support.
It would be helpful and much more convenient for me and other
folks
working on this stuff to avoid the custom build setup. I really
don't
think there is a downside here.
i know that's a pain, but that is not a reason to move code out
of the
sandbox.
so, after taking a closer look at the code i think everything looks
fine, but i am still -1 on committing this to the core codebase
right
now for 2 reasons.
1. as i said before, i think it is a little late in the release
cycle
to just be throwing this in. any code that we put in a release
we are
responsible for, so i don't like the idea of putting this code
in the
release if we think it's still experimental and want to keep it low
profile.
I disagree with that too. I probably should not have used the word
experimental.
I want to get interop testing on Atom protocol and interop
testing is
part of the development of the IETF Atom protocol itself. It's
harder to
get people to do that testing if they have to create a custom
build to
participate.
I am fine with getting interop testing, but I don't understand how
you
can think it's appropriate to suggest this after you have already
distributed 2 Roller-2.2 RCs. To me the whole RC process suggests
that
feature development has stopped and only bug fixes are being
committed.
I also don't consider this a particularly small addition to the code.
You said it's only 2 servlets, which is true, but it is still a
total of
28 new classes being added. That is a lot of code to add to a
release
during the RC process.
I do realize that this is a somewhat special case since you are
saying
it will be considered experimental and unsupported code, but I still
think we should not make it a habit to add new features to the code
during the RC process. If you would have suggested this even just a
week ago this would be a non-issue.
2. more importantly, i think we should think more carefully
about the
servlet endpoints we choose for these services. we are planning to
reform the roller url structure for Roller 3.0 and i am just
about to
send that proposal out to the dev list. since both atom services
introduce new urls which will be affected by this change i think
it's
better not to rush the decision to put them in the core codebase.
We don't have to stick with the end points we have now, so I
disagree
with this too. There are only two endpoints here, one for Atom
and one
or Admin. All other URLs are discovered at runtime. Changing the
endpoint URLs is really no big deal.
Sure, it's not hard for us to do it, but I would like to think
that the
code we put into our releases is meant to be mostly stable and not
likely to change that much. The only person this really affects is
anyone who starts using it and distributing the urls for use on their
site. Then when we change the urls those people get screwed.
Again, I am okay with this approach so long as anyone who tries to
use
it knows that it is likely to change in the future. And in
particular I
would note that it is basically assured that the urls for the service
endpoints will change.
I disagree with each of your objections, but I'm not going to
call for a
vote on this yet because of the timing. I'll respect this as a veto.
Thanks for taking a closer look at the code.
I still feel that all of my objections are quite valid, but i am
willing
to make an exception in this case and change my vote to +0 as long
as it
is clearly explained to anyone wanting to use this code that these 2
services are not supported in any way and that we may make any
changes
to them in the future, particularly changing the url endpoints.
-- Allen
- Dave
