I’m not crazy about adding date related stuff directly to the util package. I
am not thrilled that we have date/time related stuff there when we have a
datetime package they could have been placed in. Since we already have a
datetime package there and if those classes could be moved to core.time along
with the new ones I’d be OK with that. If the classes in util/datetime can’t be
moved to time then I would be ok with the new classes residing in
util/datetime.
I am not a fan of having date/time related classes in util, util/datetime and
time. To me having all the related classes in the same package is more
important than them residing in the package with the best name.
Ralph
> On Jan 26, 2018, at 5:09 PM, Remko Popma <remko.po...@gmail.com> wrote:
>
> On Sat, Jan 27, 2018 at 8:30 Remko Popma <remko.po...@gmail.com> wrote:
>
>>
>> On Jan 26, 2018, at 21:19, Remko Popma <remko.po...@gmail.com> wrote:
>>
>> I moved the new precise time-related classes to a new package core.time as
>> requested by Gary.
>>
>> Note that the preexisting time-related classes in util cannot be moved as
>> that would break user code.
>>
>>
>> I’m actually still unsure if moving these new classes to a new `core.time`
>> package is really better...
>>
>> For the Java 8 platform it made sense to have a separate new `java.time`
>> package: it contains a cohesive set of classes that represent a new way of
>> thinking about time that doesn’t interact well with the old date/time
>> related classes.
>>
>> None of these properties hold for the classes in `log4j.core.time`: our
>> classes simply extend the old classes and add some limited functionality.
>> Functionality-wise there’s nothing that justifies a new package.
>>
>> It would make sense to have a separate time package if we could put all
>> the existing Clock, NanoClock and factory classes there as well, but moving
>> those would break other people’s code.
>>
>> The result looks messy to me. We have some time classes in util and some
>> in core.time while conceptually there’s nothing different about them. The
>> more I think about it the less I like it.
>>
>> I agree with Gary that it’s not ideal and it would have been nicer to have
>> all time-related classes in a separate package, but that ship has sailed.
>> Having all time classes in util is more consistent than having only some
>> classes in a separate package...
>>
>> I’m thinking to move them back.
>>
>
> The only saving grace is that the classes in `log4j.core.time` are meant to
> emulate some functionality available in Java 8’s `java.time` package. But
> is that enough reason to put them in a separate package?
>
>
>
>>
>>
>> On Fri, Jan 26, 2018 at 8:59 PM, Remko Popma <remko.po...@gmail.com>
>> wrote:
>>
>>> I renamed DummyPreciseClock to FixedPreciseClock since it is similar to
>>> the Java 8 Clock::fixed factory method.
>>>
>>> On Thu, Jan 25, 2018 at 9:55 AM, Matt Sicker <boa...@gmail.com> wrote:
>>>
>>>> Aren't the classes in datetime copied from commons? Might be simpler to
>>>> keep that package uncluttered for easier updates.
>>>>
>>>> Also, I like the name ConstantClock or something like that. Constant is a
>>>> nice term with appropriate mathematical connotations.
>>>>
>>>> On 24 January 2018 at 12:00, Gary Gregory <garydgreg...@gmail.com>
>>>> wrote:
>>>>
>>>>> On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory <garydgreg...@gmail.com
>>>>>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma <remko.po...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I see what you mean. Hmm...
>>>>>>>
>>>>>>> FYI, this feature (LOG4J2-1883) adds the following classes. I added
>>>> them
>>>>>>> to
>>>>>>> core.util, but they could still be moved to another package:
>>>>>>> * Instant <interface>
>>>>>>> * PreciseClock <interface>
>>>>>>> * MutableInstant
>>>>>>> * DummyPreciseClock
>>>>>>>
>>>>>>
>>>>> Hi,
>>>>>
>>>>> I urge you _not_ to name this class Dummy*, that tells me nothing :-(
>>>>>
>>>>> OTOH, the Javadoc states:
>>>>>
>>>>> /**
>>>>> * Implementation of the {@code PreciseClock} interface that always
>>>> returns
>>>>> a fixed value.
>>>>> * @since 2.11
>>>>> */
>>>>>
>>>>>
>>>>> public class DummyPreciseClock implements PreciseClock {
>>>>>
>>>>> So I would call this class StaticPreciseClock (or
>>>> FixedValuePreciseClock,
>>>>> or ConstantPreciseClock)
>>>>>
>>>>> ah... better :-)
>>>>>
>>>>> Gary
>>>>>
>>>>>
>>>>>
>>>>>> * SystemMillisClock
>>>>>>>
>>>>>>> I would not consider moving the existing time-related classes,
>>>> because
>>>>>>> that
>>>>>>> would break client code. These should stay as is:
>>>>>>> * Clock <interface>
>>>>>>> * NanoClock <interface>
>>>>>>> * ClockFactory
>>>>>>> * SystemNanoClock, DummyNanoClock
>>>>>>> * CachedClock, CoarseCachedClock, SystemClock
>>>>>>>
>>>>>>> I also would not want to rename core.util.datetime; that would just
>>>>> break
>>>>>>> client code without any tangible benefit.
>>>>>>>
>>>>>>> So, we can add the new 5 classes to core.util.datetime, or create a
>>>> new
>>>>>>> package core.util.time and add them there, or leave them in
>>>> core.util.
>>>>>>> Not
>>>>>>> sure which I prefer actually, need to think about it a bit...
>>>>>>>
>>>>>>
>>>>>> For me the right place is core.time. Anything in or under "util" is
>>>> not
>>>>>> nice IMO. In Java 8 we have java.time, not java.util.time, so
>>>> core.time
>>>>>> feels right (and modern.)
>>>>>>
>>>>>> Gary
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Wed, Jan 24, 2018 at 11:54 PM, Gary Gregory <
>>>> garydgreg...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> The package core.time is starting to look like a kitchen sink.
>>>> Why not
>>>>>>> put
>>>>>>>> this new class into the existing core.util.datetime or into a new
>>>>>>> core.time
>>>>>>>> package. IMO core.util.datetime, should be core.datetime or simply
>>>>>>>> core.util.time.
>>>>>>>>
>>>>>>>> Gary
>>>>>>>>
>>>>>>>> On Wed, Jan 24, 2018 at 4:22 AM, <rpo...@apache.org> wrote:
>>>>>>>>
>>>>>>>>> Repository: logging-log4j2
>>>>>>>>> Updated Branches:
>>>>>>>>> refs/heads/LOG4J2-1883-instant-field 3c22d3d83 -> b8b519e5b
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> LOG4J2-1883 moved SystemMillisClock out of the java9 module into
>>>>> core
>>>>>>> so
>>>>>>>>> it can be used with any supported Java version
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Project:
>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
>>>>>>>>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/
>>>>>>>>> commit/b8b519e5
>>>>>>>>> Tree:
>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/
>>>>>>>> b8b519e5
>>>>>>>>> Diff:
>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/
>>>>>>>> b8b519e5
>>>>>>>>>
>>>>>>>>> Branch: refs/heads/LOG4J2-1883-instant-field
>>>>>>>>> Commit: b8b519e5b6125ca08545691a631e8b3e0a0e4cd0
>>>>>>>>> Parents: 3c22d3d
>>>>>>>>> Author: rpopma <rpo...@apache.org>
>>>>>>>>> Authored: Wed Jan 24 20:22:41 2018 +0900
>>>>>>>>> Committer: rpopma <rpo...@apache.org>
>>>>>>>>> Committed: Wed Jan 24 20:22:41 2018 +0900
>>>>>>>>>
>>>>>>>>> ------------------------------------------------------------
>>>>>>> ----------
>>>>>>>>> .../log4j/core/util/SystemMillisClock.java | 34
>>>>>>>> --------------------
>>>>>>>>> .../log4j/core/util/SystemMillisClock.java | 34
>>>>>>>> ++++++++++++++++++++
>>>>>>>>> 2 files changed, 34 insertions(+), 34 deletions(-)
>>>>>>>>> ------------------------------------------------------------
>>>>>>> ----------
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
>>>>>>>>> b8b519e5/log4j-core-java9/src/main/java/org/apache/logging/
>>>>>>>>> log4j/core/util/SystemMillisClock.java
>>>>>>>>> ------------------------------------------------------------
>>>>>>> ----------
>>>>>>>>> diff --git a/log4j-core-java9/src/main/
>>>>> java/org/apache/logging/log4j/
>>>>>>>>> core/util/SystemMillisClock.java b/log4j-core-java9/src/main/
>>>>>>>>> java/org/apache/logging/log4j/core/util/SystemMillisClock.java
>>>>>>>>> deleted file mode 100644
>>>>>>>>> index f267320..0000000
>>>>>>>>> --- a/log4j-core-java9/src/main/java/org/apache/logging/log4j/
>>>>>>>>> core/util/SystemMillisClock.java
>>>>>>>>> +++ /dev/null
>>>>>>>>> @@ -1,34 +0,0 @@
>>>>>>>>> -/*
>>>>>>>>> - * Licensed to the Apache Software Foundation (ASF) under one
>>>> or
>>>>> more
>>>>>>>>> - * contributor license agreements. See the NOTICE file
>>>> distributed
>>>>>>> with
>>>>>>>>> - * this work for additional information regarding copyright
>>>>>>> ownership.
>>>>>>>>> - * The ASF licenses this file to You under the Apache license,
>>>>>>> Version
>>>>>>>> 2.0
>>>>>>>>> - * (the "License"); you may not use this file except in
>>>> compliance
>>>>>>> with
>>>>>>>>> - * the License. You may obtain a copy of the License at
>>>>>>>>> - *
>>>>>>>>> - * http://www.apache.org/licenses/LICENSE-2.0
>>>>>>>>> - *
>>>>>>>>> - * Unless required by applicable law or agreed to in writing,
>>>>>>> software
>>>>>>>>> - * distributed under the License is distributed on an "AS IS"
>>>>> BASIS,
>>>>>>>>> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
>>>> express or
>>>>>>>>> implied.
>>>>>>>>> - * See the license for the specific language governing
>>>> permissions
>>>>>>> and
>>>>>>>>> - * limitations under the license.
>>>>>>>>> - */
>>>>>>>>> -package org.apache.logging.log4j.core.util;
>>>>>>>>> -
>>>>>>>>> -/**
>>>>>>>>> - * Implementation of the {@code Clock} interface that returns
>>>> the
>>>>>>> system
>>>>>>>>> time in millisecond granularity.
>>>>>>>>> - * @since 2.11
>>>>>>>>> - */
>>>>>>>>> -public final class SystemMillisClock implements Clock {
>>>>>>>>> -
>>>>>>>>> - /**
>>>>>>>>> - * Returns the system time.
>>>>>>>>> - * @return the result of calling {@code
>>>>>>> System.currentTimeMillis()}
>>>>>>>>> - */
>>>>>>>>> - @Override
>>>>>>>>> - public long currentTimeMillis() {
>>>>>>>>> - return System.currentTimeMillis();
>>>>>>>>> - }
>>>>>>>>> -
>>>>>>>>> -}
>>>>>>>>>
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/
>>>>>>>>> b8b519e5/log4j-core/src/main/java/org/apache/logging/log4j/
>>>>>>>>> core/util/SystemMillisClock.java
>>>>>>>>> ------------------------------------------------------------
>>>>>>> ----------
>>>>>>>>> diff --git a/log4j-core/src/main/java/
>>>>> org/apache/logging/log4j/core/
>>>>>>>> util/SystemMillisClock.java
>>>>>>>>> b/log4j-core/src/main/java/org/apache/logging/log4j/core/
>>>>>>>>> util/SystemMillisClock.java
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..f267320
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/
>>>>>>>>> util/SystemMillisClock.java
>>>>>>>>> @@ -0,0 +1,34 @@
>>>>>>>>> +/*
>>>>>>>>> + * Licensed to the Apache Software Foundation (ASF) under one
>>>> or
>>>>> more
>>>>>>>>> + * contributor license agreements. See the NOTICE file
>>>> distributed
>>>>>>> with
>>>>>>>>> + * this work for additional information regarding copyright
>>>>>>> ownership.
>>>>>>>>> + * The ASF licenses this file to You under the Apache license,
>>>>>>> Version
>>>>>>>> 2.0
>>>>>>>>> + * (the "License"); you may not use this file except in
>>>> compliance
>>>>>>> with
>>>>>>>>> + * the License. You may obtain a copy of the License at
>>>>>>>>> + *
>>>>>>>>> + * http://www.apache.org/licenses/LICENSE-2.0
>>>>>>>>> + *
>>>>>>>>> + * Unless required by applicable law or agreed to in writing,
>>>>>>> software
>>>>>>>>> + * distributed under the License is distributed on an "AS IS"
>>>>> BASIS,
>>>>>>>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
>>>> express or
>>>>>>>>> implied.
>>>>>>>>> + * See the license for the specific language governing
>>>> permissions
>>>>>>> and
>>>>>>>>> + * limitations under the license.
>>>>>>>>> + */
>>>>>>>>> +package org.apache.logging.log4j.core.util;
>>>>>>>>> +
>>>>>>>>> +/**
>>>>>>>>> + * Implementation of the {@code Clock} interface that returns
>>>> the
>>>>>>> system
>>>>>>>>> time in millisecond granularity.
>>>>>>>>> + * @since 2.11
>>>>>>>>> + */
>>>>>>>>> +public final class SystemMillisClock implements Clock {
>>>>>>>>> +
>>>>>>>>> + /**
>>>>>>>>> + * Returns the system time.
>>>>>>>>> + * @return the result of calling {@code
>>>>>>> System.currentTimeMillis()}
>>>>>>>>> + */
>>>>>>>>> + @Override
>>>>>>>>> + public long currentTimeMillis() {
>>>>>>>>> + return System.currentTimeMillis();
>>>>>>>>> + }
>>>>>>>>> +
>>>>>>>>> +}
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Matt Sicker <boa...@gmail.com>
>>>>
>>>
>>>
>>
