Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Ralph Goers]

> On Jan 26, 2018, at 8:54 PM, Remko Popma  wrote:
> 
> 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.

We definitely should be moving in that direction. Someday we will want to turn 
log4j-core into a “real” Java module. Doing that will require that only 
packages containing classes meant to be public be exported. So moving private 
classes to “private” directories is a step in that direction.

Ralph

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Remko Popma]

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 
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  wrote:
> >
> > On Sat, Jan 27, 2018 at 8:30 Remko Popma  wrote:
> >
> >>
> >> On Jan 26, 2018, at 21:19, Remko Popma  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 
> >> 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  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 
>  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  >
> >> 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 
> >>> * PreciseClock 
> >>> * MutableInstant
> >>> * DummyPreciseClock
> >>>
> >>
> > Hi,
> 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Ralph Goers]

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  wrote:
> 
> On Sat, Jan 27, 2018 at 8:30 Remko Popma  wrote:
> 
>> 
>> On Jan 26, 2018, at 21:19, Remko Popma  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 
>> 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  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 
 wrote:
 
> On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory  
> wrote:
> 
>> 
>> 
>> On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
>> 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 
>>> * PreciseClock 
>>> * 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 
>>> * NanoClock 
>>> * 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 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Remko Popma]

On Sat, Jan 27, 2018 at 8:30 Remko Popma  wrote:

>
> On Jan 26, 2018, at 21:19, Remko Popma  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 
> 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  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 
>>> wrote:
>>>
>>> > On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory >> >
>>> > wrote:
>>> >
>>> > >
>>> > >
>>> > > On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
>>> > > 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 
>>> > >> * PreciseClock 
>>> > >> * 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 
>>> > >> * NanoClock 
>>> > >> * 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 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Remko Popma]

> On Jan 26, 2018, at 21:19, Remko Popma  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  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  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  wrote:
>>> 
>>> > On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory 
>>> > wrote:
>>> >
>>> > >
>>> > >
>>> > > On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
>>> > > 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 
>>> > >> * PreciseClock 
>>> > >> * 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 
>>> > >> * NanoClock 
>>> > >> * 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 
>>> > >> 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,  wrote:
>>> > >> >
>>> > >> > > Repository: logging-log4j2
>>> > >> > > Updated Branches:
>>> > >> > >   refs/heads/LOG4J2-1883-instant-field 3c22d3d83 -> b8b519e5b
>>> > >> > >
>>> > >> > >

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Matt Sicker]

Sounds good to me!

On 26 January 2018 at 06:19, Remko Popma  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.
>
> On Fri, Jan 26, 2018 at 8:59 PM, Remko Popma 
> 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  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 
> 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  >
> >> > > 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 
> >> > >> * PreciseClock 
> >> > >> * 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 
> >> > >> * NanoClock 
> >> > >> * 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,  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/r
> >> epos/asf/logging-log4j2/repo
> >> > >> > > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/
> >> > >> > > commit/b8b519e5
> >> > >> > > Tree: http://git-wip-us.apache.org/r
> >> epos/asf/logging-log4j2/tree/
> >> > >> > b8b519e5
> >> > >> > > Diff: http://git-wip-us.apache.org/r
> >> epos/asf/logging-log4j2/diff/
> >> > >> > b8b519e5
> >> > >> > >
> >> > >> > > Branch: refs/heads/LOG4J2-1883-instant-field
> >> > >> > > Commit: b8b519e5b6125ca08545691a631e8b3e0a0e4cd0
> >> > >> > > Parents: 3c22d3d
> >> > >> > > Author: rpopma 
> >> > >> > > Authored: Wed Jan 24 20:22:41 2018 +0900
> >> > >> > > Committer: rpopma 
> >> > >> > > Committed: Wed Jan 24 20:22:41 2018 +0900
> >> > >> > >
> >> > >> > > 
> 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Remko Popma]

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.

On Fri, Jan 26, 2018 at 8:59 PM, Remko Popma  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  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  wrote:
>>
>> > On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory 
>> > wrote:
>> >
>> > >
>> > >
>> > > On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
>> > > 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 
>> > >> * PreciseClock 
>> > >> * 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 
>> > >> * NanoClock 
>> > >> * 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,  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/r
>> epos/asf/logging-log4j2/repo
>> > >> > > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/
>> > >> > > commit/b8b519e5
>> > >> > > Tree: http://git-wip-us.apache.org/r
>> epos/asf/logging-log4j2/tree/
>> > >> > b8b519e5
>> > >> > > Diff: http://git-wip-us.apache.org/r
>> epos/asf/logging-log4j2/diff/
>> > >> > b8b519e5
>> > >> > >
>> > >> > > Branch: refs/heads/LOG4J2-1883-instant-field
>> > >> > > Commit: b8b519e5b6125ca08545691a631e8b3e0a0e4cd0
>> > >> > > Parents: 3c22d3d
>> > >> > > Author: rpopma 
>> > >> > > Authored: Wed Jan 24 20:22:41 2018 +0900
>> > >> > > Committer: rpopma 
>> > >> > > 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(-)
>> > >> > > 
>> > >> --
>> > >> > >
>> > >> > >
>> > >> > > 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Remko Popma]

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  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  wrote:
>
> > On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory 
> > wrote:
> >
> > >
> > >
> > > On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
> > > 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 
> > >> * PreciseClock 
> > >> * 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 
> > >> * NanoClock 
> > >> * 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,  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 
> > >> > > Authored: Wed Jan 24 20:22:41 2018 +0900
> > >> > > Committer: rpopma 
> > >> > > 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/
> > >> > > 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Matt Sicker]

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

> On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory 
> wrote:
>
> >
> >
> > On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
> > 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 
> >> * PreciseClock 
> >> * 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 
> >> * NanoClock 
> >> * 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 
> >> 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,  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 
> >> > > Authored: Wed Jan 24 20:22:41 2018 +0900
> >> > > Committer: rpopma 
> >> > > 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..000
> >> > > --- 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 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Gary Gregory]

On Wed, Jan 24, 2018 at 10:55 AM, Gary Gregory 
wrote:

>
>
> On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma 
> 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 
>> * PreciseClock 
>> * 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 
>> * NanoClock 
>> * 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 
>> 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,  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 
>> > > Authored: Wed Jan 24 20:22:41 2018 +0900
>> > > Committer: rpopma 
>> > > 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..000
>> > > --- 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

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Gary Gregory]

On Wed, Jan 24, 2018 at 8:58 AM, Remko Popma  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 
> * PreciseClock 
> * MutableInstant
> * DummyPreciseClock
> * SystemMillisClock
>
> I would not consider moving the existing time-related classes, because that
> would break client code. These should stay as is:
> * Clock 
> * NanoClock 
> * 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 
> 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,  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 
> > > Authored: Wed Jan 24 20:22:41 2018 +0900
> > > Committer: rpopma 
> > > 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..000
> > > --- 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 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Remko Popma]

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 
* PreciseClock 
* MutableInstant
* DummyPreciseClock
* SystemMillisClock

I would not consider moving the existing time-related classes, because that
would break client code. These should stay as is:
* Clock 
* NanoClock 
* 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...



On Wed, Jan 24, 2018 at 11:54 PM, Gary Gregory 
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,  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 
> > Authored: Wed Jan 24 20:22:41 2018 +0900
> > Committer: rpopma 
> > 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..000
> > --- 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/
> > 

Re: logging-log4j2 git commit: LOG4J2-1883 moved SystemMillisClock out of the java9 module into core so it can be used with any supported Java version

[Gary Gregory]

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,  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 
> Authored: Wed Jan 24 20:22:41 2018 +0900
> Committer: rpopma 
> 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..000
> --- 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 000..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;
> +
> +/**