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>
>>>
>>
>>
>
