Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate o.a.l.l.c.a.r.a.Duration class for removal #2425

Merged
merged 6 commits into from
Mar 30, 2024
Merged

Conversation

ppkarwasz
Copy link
Contributor

We deprecate the o.a.l.l.c.a.r.a.Duration and replace it with java.time.Duration.

Copy link
Member

@vy vy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have rather removed action.Duration, but anyway...

@ppkarwasz ppkarwasz merged commit b1c9cd8 into 2.x Mar 30, 2024
6 of 7 checks passed
@ppkarwasz ppkarwasz deleted the deprecate_duration branch March 30, 2024 22:54
@ppkarwasz ppkarwasz added this to the 2.24.0 milestone Aug 16, 2024
@norrisjeremy
Copy link

Hi @ppkarwasz,

Can you update the release notes for 2.24.0 to document the fact that this deprecation also means that the relaxed syntax of with 'P' and 'T' optional is also thus deprecated?
This is not clearly defined in the release notes and thus means consumers of o.a.l.l.c.a.r.a.Duration that naively migrate to java.time.Duration can thus suddenly encounter java.time.format.DateTimeParseException's for strings that could previously be parsed without issue.

Thanks,
Jeremy

@norrisjeremy
Copy link

Hi @ppkarwasz,

Can you update the release notes for 2.24.0 to document the fact that this deprecation also means that the relaxed syntax of with 'P' and 'T' optional is also thus deprecated? This is not clearly defined in the release notes and thus means consumers of o.a.l.l.c.a.r.a.Duration that naively migrate to java.time.Duration can thus suddenly encounter java.time.format.DateTimeParseException's for strings that could previously be parsed without issue.

Thanks, Jeremy

I also am not sure if it would be worthwhile calling out to users in the Release Notes that configuration elements such as <IfLastModified age="7d"/> are also thus deprecated and should be updated to syntax such as <IfLastModified age="P7D"/>?

@vy
Copy link
Member

vy commented Sep 12, 2024

I also am not sure if it would be worthwhile calling out to users in the Release Notes that configuration elements such as <IfLastModified age="7d"/> are also thus deprecated and should be updated to syntax such as <IfLastModified age="P7D"/>?

@norrisjeremy, to keep the backward compatibility, Log4j 2 still uses the (deprecated) Log4j Duration class to read IfLastModified#age – the new j.t.Duration syntax is not supported yet. We only marked the class and its usages as deprecated to warn users accessing those fields programmatically, etc. But you have a point: Log4j 3 completely switches to j.t.Duration and this will break incompatible IfLastModified configurations. That said, Log4j 3 will be a big bang anyway... 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants