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

Force selection of calendar or fixed intervals in date histo aggregations #3988

Merged
merged 2 commits into from
Aug 9, 2019

Conversation

codebrain
Copy link
Contributor

Copy link
Contributor

@russcam russcam left a comment

Choose a reason for hiding this comment

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

LGTM.

Unsure of the best way to handle Interval -> CalendarInterval/FixedInterval with respect to usage tests; the pragma directives allow the tests to work for all 7.x versions, but it does mean that the use of Interval will be in documentation for all 7.x versions, because documentation is not generated for each minor version.

src/Tests/Tests.Reproduce/GithubIssue3673.cs Outdated Show resolved Hide resolved
Copy link
Member

@Mpdreamz Mpdreamz left a comment

Choose a reason for hiding this comment

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

LGTM, ideally the pragma disable/restore is only around the offending line setting .Interval() though

@codebrain codebrain merged commit 802c570 into 7.2 Aug 9, 2019
@russcam russcam deleted the feature/7.2/calendar-interval branch September 1, 2019 22:41
russcam pushed a commit that referenced this pull request Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants