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

Use weekyear for WEEK_YEAR format name #60855

Closed
wants to merge 1 commit into from

Conversation

russcam
Copy link
Contributor

@russcam russcam commented Aug 7, 2020

Relates: #60707

With the deprecation of camel case names in #59555,
the format "weekyear" was deprecated in favour of
"week_year". This is inconsistent with other weekyear
formats however, that use "weekyear", for example

WEEKYEAR_WEEK("weekyear_week"),
WEEKYEAR_WEEK_DAY("weekyear_week_day"),

This commit keeps "weekyear" as the snake case name
for the enum constant FormatNames.WEEK_YEAR. In addition,
the format for DateFormatter WEEK_YEAR is changed to
"weekyear".

If agreed, this PR would need to be backported to 7.9 because #59555 has been merged to 7.9.

Relates: elastic#60707

With the deprecation of camel case names in elastic#59555,
the format "weekyear" was deprecated in favour of
"week_year". This is inconsistent with other weekyear
formats however, that use "weekyear", for example

```
WEEKYEAR_WEEK("weekyear_week"),
WEEKYEAR_WEEK_DAY("weekyear_week_day"),
```

This commit keeps "weekyear" as the snake case name
for the enum constant FormatNames.WEEK_YEAR. In addition,
the format for DateFormatter WEEK_YEAR is changed to
"weekyear".
@rjernst
Copy link
Member

rjernst commented Aug 13, 2020

I think it is already too late to make 7.9, and this change looks like it would be breaking? We need to properly deprecate usages of the snake_case version.

Copy link
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

maybe we could deprecate in 7.x and also add a new WEEKYEAR("weekyear") value?

@pgomulka
Copy link
Contributor

btw I also plan to review and remove some of these formats. I only briefly mentioned this here #58719
WEEKYEAR is one of them to be removed in my book. I don't find it to be massively useful as it is easier to just use a 'YYYY' :)

@pgomulka
Copy link
Contributor

pgomulka commented Oct 7, 2020

closing in favour of #63384

@pgomulka pgomulka closed this Oct 7, 2020
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