I guess the classes in util/datetime can be moved to the new time package.
Note that these are public classes so there is a probability that a user is
using these classes directly and this move will break their code, but if we
judge this probability to be low and the argument could be made that these
classes are private for Log4j2 internal use.
The same argument could be made for many of the time-related implementation
classes in util. The interfaces (Clock and NanoClock) are published and
documented extension points for users to plug into Log4j2. These interfaces
need to stay in util.
Perhaps it would be wise to take this opportunity to create two packages:
core/time and core/time/internal, where everything in the internal package
should not be directly used by users and is free for us to modify at any
time.
On Sat, Jan 27, 2018 at 9:18 AM, Ralph Goers <ralph.go...@dslextreme.com>
wrote:
> 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>
> >>>>
> >>>
> >>>
> >>
>
>
>
