> Dave Rolsky wrote:
>>Why borg the Windows and HP-UX code into this distro instead of letting
>>other maintainers handle this?
>
> I don't think there's significant value in having separate distros here.
> Having a single distro has the distinct advantages that the behaviour
> is consistent: you just install the one distro, and its behaviour isn't
> affected by whether you installed another one. It also ultimately gives
> a conceptual consistency across platforms, by having one person apply a
> single philosophy to each platform. The only real advantage of separate
> distros is to make it easier for platform-specific developers to work
> on their own platform's code, but in the single-distro system they can
> still submit patches to the single maintainer.
I'd like to hear from David Pinkowitz and Olivier Mengue on this. I don't
care
too much about how things are split up, I just want to make sure it's easy
for
them to get updates out to the world at large.
>>If you want to document DateTime::TimeZone as not a class, I think having
>>a documented "DateTime::TimeZone::new" function is really weird.
>
> Yeah, it's a bit odd. I've just left the names the same, so they're all
> named from the point of view of class methods. "is_valid_name" is a poor
> standalone function name; it should be "is_valid_zone_name" or something.
> I didn't want to invent more new stuff than necessary initially.
I'd be fine with new names for utility subs like this. Obviously there'd have
to be a deprecation period.
The main reason we need DT::TZ->new to work is so that the new DT::TZ distro
is compatible with existing DateTime.pm installations. I'd like people to be
able DT::TZ without a conflict on an existing DateTime.pm.
> If we're going to have proper function names, I think we should go
> the whole way and make them exportable too. I think just "new" and
> "is_valid_name" should have this treatment, because the other functions
> are just additional ways of accessing functionality that really belongs
> elsewhere. For "new" I prefer the name "lookup_zone" over "load_zone",
> because there's caching and so on involved, so it's not necessarily
> actually loading anything.
I'm totally fine with "lookup_zone".
>>Obviously, the old DT::TZ->new behavior would need to work too, but it
>>doesn't need to be documented, and once this code is released, I can
>>adjust DateTime.pm to use the new APIs.
>
> I think if it's to be supported indefinitely (which it must be) then it
> needs to be documented, but it doesn't need to be recommended. It can
> be explicitly deprecated, along with the catalogue and offset functions.
It can be documented at the end of the docs under a section on deprecated
backwards compatibility code.
>> One of them was clearly a bug in the
>>test suite (expecting 0 for a boolean instead of any false value).
>
> Where functions were documented to return truth values, I presumed
> that any truth value would be acceptable. DT:TZ:Alias is different:
> it documented that it returns 1 or undef, so I preserved that behaviour.
Yeah, I fixed the test suite. This isn't a big issue, since installing the
new
DT::TZ won't break an existing DateTime.pm because of this. It just broke the
test suite. When you release the new DT::TZ I'll release a new DateTime.pm
with fixed tests.
>>The exception that's thrown for an invalid local time is different with
>>this new version of the library.
>
> That'd be from DT:TZ:Tzfile. I think you're referring to this difference:
>
> $ perl -MDateTime::TimeZone -MDateTime::TimeZone::Olson -MDateTime -we
> '$dt =
> DateTime->new(year=>2011,month=>3,day=>27,hour=>1,minute=>30,time_zone=>"UTC");
> foreach my $z (DateTime::TimeZone->new(name=>"Europe/London"),
> DateTime::TimeZone::Olson::olson_tz("Europe/London")) { eval {
> $z->offset_for_local_datetime($dt) }; print $@; }'
> Invalid local time for date in time zone: Europe/London
> non-existent local time due to offset change at -e line 1
>
> It's possible that something's depending on the exact format of the old
> DT:TZ error message, and it's also possible that something's depending
> on the exact format of the DT:TZ:Tzfile message. I'm not really happy
> about changing DT:TZ:Tzfile here, especially since its error message is
> more descriptive and better punctuated than the old DT:TZ one.
>
> I did wonder a bit, while writing the new DT:TZ, whether the spelling of
> error messages would be a compatibility issue. (I didn't think about
> it for the specific message discussed above, since that was ages ago
> in a different distro, which backward compatibility wasn't a concern.)
> I came to the conclusion that I'd like to see whether any real code
> actually broke because of it. It's difficult to decide what lengths we
> should go to for backcompat without having any information of that nature.
I think the message from TzFile is actually less informative. Really, I think
the ideal would be to include the zone and the invalid local time in the
message.
I guess I'm okay with changing the message as long as no one on the list
speaks up about it. Like the boolean issue, it only breaks tests, so it won't
actually break an existing DateTime.pm install.
>>The DateTime::TimeZone::Catalog module needs to exist, if only for
>>documentation purposes.
>
> Looking at where your line of questioning goes, it looks like you're
> referring to having the list of timezones in the documentation. I don't
> think this belongs in documentation, being volatile data. I'm not happy
> about the idea of generating such a document in T:OTZ:D. I think if you
> want to make this readily visible it would be better to ship code that
> will generate the POD from the underlying data that it gets from T:OTZ:D.
> (Actually, like the catalogue functions in the new DT:TZ, it ought to
> get the data from DT:TZ:Olson, which provides another interface to the
> data in T:OTZ:D, for modules that are thinking at a higher level.)
There's two issues here.
First, the module needs to just exist in the new distro. I think people might
be relying on it for docs. If the new distro doesn't replace it then they'll
be left with out of date docs, which is no good.
As to how and when these docs get generated, I don't really care, I just want
the docs to be available after the distro is installed. If you want to do
that
as part of the install process that's fine.
> There'll be a bit of an issue where the RDN exceeds the range of 32-bit
> integer arithmetic, in about 11.8 million years. Existing DT:TZ would
> throw a fit (well, eat a lot of memory) with that sort of date. I can
> beef up DT:TZ:Tzfile for that if you're worried, but I suspect DateTime
> itself would also break at that point.
Yeah, I'm not going to worry about 11.8 million years out ;)
>>The COMPAT document you attached is good, but it's missing this key
>> point.
>
> I was wondering if COMPAT should be included in the distro, to help
> DT:TZ API users with the transition. What do you think?
Yeah, I think shipping it would be a good idea. You can make a note of it in
the Changes file.
>>I'd really like to see this code in a public repo before it's released,
>>along with the various other modules this depends on.
>
> Hmm. I really ought to get on with switching my personal projects to git.
> I've been putting that off for ages.
>
> I have my own public git server. I looked at github recently, and I
> can't accept its terms of service (and neither should you).
Whatever works for you. As long as it's publicly available and in the distro
metadata, I'm happy. I just don't want the project to become less visible
after you take it over.
>>As far as maintenance, I would like to have comaint, although I'm not
>>sure how much good that will really do me since all the real work is done
>>in other distros anyway. But it'll make me feel better ;)
>
> I also have difficulty imagining how it would be useful, but I have
> little experience with this. One other person has co-maint on one of
> my modules, the first one that I span off from $ork, and he's never
> used it. If I drop off the net for an extended period without making
> maintenance arrangements, you can always go the takeover route, and
> you could probably get a fast-tracked co-maint that way if there were
> something requiring immediate attention. So I don't see any situation
> where you'd validly use co-maint that you have only via this handover
> arrangement, but I can't sensibly deny it.
Yeah, that's a good point. I guess I'm fine with not having comaint.
-dave
