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

Add feature flag to stop defaulting null datetime filter args to current time #1067

Merged
merged 5 commits into from
May 3, 2023

Conversation

boulter
Copy link
Contributor

@boulter boulter commented May 2, 2023

This is like #1065, but uses the new feature flags instead of adding to JInjavaConfig.

#1066 should be merged first.

@boulter boulter changed the title add features flags to jinjava config Add config to stop defaulting null datetime filter args to current time May 2, 2023
@boulter boulter marked this pull request as draft May 2, 2023 22:29
@jasmith-hs jasmith-hs changed the base branch from master to jboulter-add-features-flags-to-jinjava-config May 3, 2023 13:06
.getActivationStrategy(FIXED_DATE_TIME_FILTER_NULL_ARG);

if (feat.isActive()) {
var = ((DateTimeFeatureActivationStrategy) feat).getActivateAt();
Copy link
Contributor

Choose a reason for hiding this comment

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

These are not safe casts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general, true, but for this particular feature it will be.

Copy link
Contributor

@jasmith-hs jasmith-hs May 3, 2023

Choose a reason for hiding this comment

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

If the feature-flag is changed to FeatureStrategies.ACTIVE, this will blow up at runtime

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. But for compatibility reasons, this has to return a valid date. So it's either a casting exception or some other exception.

Base automatically changed from jboulter-add-features-flags-to-jinjava-config to master May 3, 2023 13:22
@boulter boulter marked this pull request as ready for review May 3, 2023 13:23
@@ -41,7 +42,7 @@ public void testSafeStringCanBeEscaped() {
.isInstanceOf(SafeString.class);
}

@Test
@Ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

sometimes this is not faster when building.

@boulter boulter changed the title Add config to stop defaulting null datetime filter args to current time Add feature to stop defaulting null datetime filter args to current time May 3, 2023
@boulter boulter changed the title Add feature to stop defaulting null datetime filter args to current time Add feature flag to stop defaulting null datetime filter args to current time May 3, 2023
@boulter boulter merged commit ddb9aba into master May 3, 2023
@boulter boulter deleted the null-date-time-feature branch May 3, 2023 21:39
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