You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/31 17:11:58 UTC

[GitHub] [commons-lang] ugonen opened a new pull request #688: LANG-1637: fix 2 digit week year formatting

ugonen opened a new pull request #688:
URL: https://github.com/apache/commons-lang/pull/688


   https://issues.apache.org/jira/browse/LANG-1637


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] michael-o commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550994951



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       @garydgregory WDYT?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory removed a comment on pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory removed a comment on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753622387


   @ugonen 
   It looks like you have FOUR PRs about this issue. Please close the irrelevant PRs so we know where to look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] coveralls commented on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753474295


   
   [![Coverage Status](https://coveralls.io/builds/36039362/badge)](https://coveralls.io/builds/36039362)
   
   Coverage increased (+0.0009%) to 95.013% when pulling **bc0354f22e1d42a2a08016fd2d577f1e65d9d35c on ugonen:LANG-1637-fix-week-year-two-digits** into **f5c86f6d4e8d9023f9e023fb1cd09ef47c8fd1cf on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory closed pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory closed pull request #688:
URL: https://github.com/apache/commons-lang/pull/688


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] coveralls edited a comment on pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753474295


   
   [![Coverage Status](https://coveralls.io/builds/36040263/badge)](https://coveralls.io/builds/36040263)
   
   Coverage increased (+0.0009%) to 95.013% when pulling **b1ad8ae3ea910e37cfc69b18d70158861942739d on ugonen:LANG-1637-fix-week-year-two-digits** into **f5c86f6d4e8d9023f9e023fb1cd09ef47c8fd1cf on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a change in pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550682944



##########
File path: src/test/java/org/apache/commons/lang3/time/FastDatePrinterTest.java
##########
@@ -432,4 +432,15 @@ public void testDayNumberOfWeek() {
         calendar.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY);
         assertEquals("7", printer.format(calendar.getTime()));
     }
+
+    @DefaultLocale(language = "en", country = "US")
+    @DefaultTimeZone("America/New_York")
+    @Test
+    public void testWeekYear() {
+        final GregorianCalendar cal = new GregorianCalendar(2020, 12, 31, 0, 0, 0);
+        final DatePrinter printer4Digits = getInstance("YYYY");
+        final DatePrinter printer2Digits = getInstance("YY");

Review comment:
       Hi,
   Thank you for your PR.
   What about "YYY"?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a change in pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550882167



##########
File path: src/test/java/org/apache/commons/lang3/time/FastDatePrinterTest.java
##########
@@ -432,4 +432,15 @@ public void testDayNumberOfWeek() {
         calendar.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY);
         assertEquals("7", printer.format(calendar.getTime()));
     }
+
+    @DefaultLocale(language = "en", country = "US")
+    @DefaultTimeZone("America/New_York")
+    @Test
+    public void testWeekYear() {
+        final GregorianCalendar cal = new GregorianCalendar(2020, 12, 31, 0, 0, 0);
+        final DatePrinter printer4Digits = getInstance("YYYY");
+        final DatePrinter printer2Digits = getInstance("YY");

Review comment:
       Please add code in your new test for Y and YYY.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] ugonen commented on a change in pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
ugonen commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550896126



##########
File path: src/test/java/org/apache/commons/lang3/time/FastDatePrinterTest.java
##########
@@ -432,4 +432,15 @@ public void testDayNumberOfWeek() {
         calendar.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY);
         assertEquals("7", printer.format(calendar.getTime()));
     }
+
+    @DefaultLocale(language = "en", country = "US")
+    @DefaultTimeZone("America/New_York")
+    @Test
+    public void testWeekYear() {
+        final GregorianCalendar cal = new GregorianCalendar(2020, 12, 31, 0, 0, 0);
+        final DatePrinter printer4Digits = getInstance("YYYY");
+        final DatePrinter printer2Digits = getInstance("YY");

Review comment:
       added, thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory merged pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #688:
URL: https://github.com/apache/commons-lang/pull/688


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753325246


   Closing per the comment in the ticket https://issues.apache.org/jira/browse/LANG-1637


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] michael-o commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550939708



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       I am not convinced by this because it patches the wrong spot.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] coveralls edited a comment on pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753474295


   
   [![Coverage Status](https://coveralls.io/builds/36040275/badge)](https://coveralls.io/builds/36040275)
   
   Coverage decreased (-0.01%) to 95.001% when pulling **b1ad8ae3ea910e37cfc69b18d70158861942739d on ugonen:LANG-1637-fix-week-year-two-digits** into **f5c86f6d4e8d9023f9e023fb1cd09ef47c8fd1cf on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753474335


   https://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] michael-o commented on pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
michael-o commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753496059


   Will have a look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753492355


   @michael-o The PR seems OK to me now. WDYT?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] ugonen commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
ugonen commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550986137



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       the name TwoDigitYearField implies this class encapsulates formatting of year field to a 2 digit string. as such, it seems reasonable for it to have the logic to take any year integer and map it to a decade followed by a year digits.
   you can also see that the other method in this class 
   `void appendTo(Appendable buf, Calendar calendar) throws IOException`
   implementing the Rule interface, does the same




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753473812


   Reopening, I was going by the "invalid" comment.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] michael-o commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r551010819



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       I think here: https://issues.apache.org/jira/browse/LANG-1637?focusedCommentId=17257625&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17257625 and this method must throw IAE `if (value >= 100)`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a change in pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r550682944



##########
File path: src/test/java/org/apache/commons/lang3/time/FastDatePrinterTest.java
##########
@@ -432,4 +432,15 @@ public void testDayNumberOfWeek() {
         calendar.set(Calendar.DAY_OF_WEEK, Calendar.SUNDAY);
         assertEquals("7", printer.format(calendar.getTime()));
     }
+
+    @DefaultLocale(language = "en", country = "US")
+    @DefaultTimeZone("America/New_York")
+    @Test
+    public void testWeekYear() {
+        final GregorianCalendar cal = new GregorianCalendar(2020, 12, 31, 0, 0, 0);
+        final DatePrinter printer4Digits = getInstance("YYYY");
+        final DatePrinter printer2Digits = getInstance("YY");

Review comment:
       What about "YYY"?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory edited a comment on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753474335


   - https://docs.oracle.com/javase/8/docs/api/java/text/SimpleDateFormat.html
   - https://docs.oracle.com/javase/8/docs/api/java/util/GregorianCalendar.html#week_year


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r552716201



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       Well... which way is this one going to go? @ugonen Are you going to update your PR or @michael-o do you want to provide a different PR?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] michael-o commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
michael-o commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r552917466



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       I think for the ease of use we have to use this one because my solution leaks abstraction. It makes too many assumptions about in the delegated `NumberRule`




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on a change in pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#discussion_r551010473



##########
File path: src/main/java/org/apache/commons/lang3/time/FastDatePrinter.java
##########
@@ -1118,7 +1118,7 @@ public void appendTo(final Appendable buffer, final Calendar calendar) throws IO
          */
         @Override
         public final void appendTo(final Appendable buffer, final int value) throws IOException {
-            appendDigits(buffer, value);
+            appendDigits(buffer, value % 100);

Review comment:
       Hi @michael-o 
   Earlier you said "I am not convinced by this because it patches the wrong spot." implying there was a "right" spot, but where would that be?
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] ugonen commented on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
ugonen commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753470530


   @garydgregory  can you explain why you closed this PR ? 
   formatting date with year 31 Dec 2020 with pattern YY should result in '21' but instead you get 'รบ1'  (before the fix) which is clearly a bug


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory commented on pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753622387


   @ugonen 
   It looks like you have FOUR PRs about this issue. Please close the irrelevant PRs so we know where to look.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] coveralls edited a comment on pull request #688: [LANG-1637] Fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753474295


   
   [![Coverage Status](https://coveralls.io/builds/36040229/badge)](https://coveralls.io/builds/36040229)
   
   Coverage increased (+0.0009%) to 95.013% when pulling **fd4bb4ab89678ce29fd18d8fc20bcfb57f146a8f on ugonen:LANG-1637-fix-week-year-two-digits** into **f5c86f6d4e8d9023f9e023fb1cd09ef47c8fd1cf on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [commons-lang] garydgregory removed a comment on pull request #688: LANG-1637: fix 2 digit week year formatting

Posted by GitBox <gi...@apache.org>.
garydgregory removed a comment on pull request #688:
URL: https://github.com/apache/commons-lang/pull/688#issuecomment-753325246


   Closing per the comment in the ticket https://issues.apache.org/jira/browse/LANG-1637


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org