Use weekyear for consistency #60707
Closed
Use weekyear for consistency #60707
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Relates: elastic#60044 and elastic#59555 (review) With the removal of camel case format names, the format name for `WEEK_YEAR` has been set to `week_year`, but this is inconsistent with the usage in other formats related to weekyear e.g. ``` WEEKYEAR_WEEK("weekyear_week"), WEEKYEAR_WEEK_DAY("weekyear_week_day"), ``` This commit - replaces the snakecase format `week_year` with `weekyear` - renames enum constants from `WEEK_YEAR` to `WEEKYEAR` for consistency with other constants related to weekyear.
russcam
added a commit
to russcam/elasticsearch
that referenced
this pull request
Aug 7, 2020
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".
|
This looks fine, but we should first have a PR to add the deprecation. It should go through the master branch first, and be backported, then this change can apply to master only. |
pgomulka
suggested changes
Aug 17, 2020
There was a problem hiding this comment.
it looks good to me, but I wonder if we should add a WEEKYEAR in 7.x, deprecate WEEK_YEAR in 7.x and remove WEEK_YEAR in master?
#60855 (review)
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 6, 2020
week_year is misleading as the formatter only has a weekyear. A field corresponsing to 'Y'. 'weekyear' should be used instead relates elastic#60707
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 6, 2020
week_year is misleading as the formatter only has a weekyear. A field corresponsing to 'Y'. 'weekyear' should be used instead relates elastic#60707
pgomulka
added a commit
that referenced
this pull request
Oct 7, 2020
week_year is misleading as the formatter only has a weekyear. A field corresponding to 'Y'. 'weekyear' should be used instead relates #60707
pgomulka
added a commit
that referenced
this pull request
Oct 7, 2020
pgomulka
added a commit
to pgomulka/elasticsearch
that referenced
this pull request
Oct 7, 2020
week_year is replaced by weekyear. This is a single field YYYY date format. relates elastic#60707
|
closing in favour of #63384 |
pgomulka
added a commit
that referenced
this pull request
Oct 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates: #60044 and #59555 (review)
With the removal of camel case format names,
the format name for
WEEK_YEARhas been setto
"week_year", but this is inconsistent with the usagein other formats related to weekyear e.g.
This PR
"week_year"with"weekyear"WEEK_YEARtoWEEKYEARfor consistencywith other constants related to weekyear.
I can open another PR to address the deprecation change in 7.x/7.9 related to #59555