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