You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2020/08/13 20:23:32 UTC

[GitHub] [orc] findepi opened a new pull request #538: Fix Julian/Gregorian cutoff in a comment

findepi opened a new pull request #538:
URL: https://github.com/apache/orc/pull/538


   


----------------------------------------------------------------
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] [orc] omalley commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
omalley commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r474309885



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       Java hasn't changed its behavior, we've just become more aware of it.
   
   Does making the text "Written using the hybrid Julian/Gregorian calendar with a cutover point in October 1582" make the intent clear?




----------------------------------------------------------------
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] [orc] omalley closed pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
omalley closed pull request #538:
URL: https://github.com/apache/orc/pull/538


   


----------------------------------------------------------------
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] [orc] dongjoon-hyun commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r470280985



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       I'm wondering if the whole sentence makes sense. `The Java default calendar changes from Julian to Gregorian in October 1582`? It sounds like `Java` changes it in `October 1581`, doesn't it? Technically, 1582 is just a year when Gregorian was introduced.
   
   Shall we remove it (the whole sentence) simply?




----------------------------------------------------------------
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] [orc] findepi commented on pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
findepi commented on pull request #538:
URL: https://github.com/apache/orc/pull/538#issuecomment-678131503


   AC


----------------------------------------------------------------
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] [orc] findepi commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
findepi commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r470250004



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       this is the only occurrence of "1583" i could find in the codebase




----------------------------------------------------------------
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] [orc] dongjoon-hyun commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r471672045



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       @findepi 
   We had better be specific to Java version because Apache Orc still supports Java 7. I'm not aware of any change in Java 7. Do you have any reference saying Java 7 changes it's default calendar or time-related stuff? IIRC, everything starts at Java 8.
   - https://github.com/apache/orc/blame/master/README.md#L47




----------------------------------------------------------------
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] [orc] dongjoon-hyun commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r470244223



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       Hi, @findepi . Is this the last one? Do we have more obsolete comments?




----------------------------------------------------------------
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] [orc] findepi commented on pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
findepi commented on pull request #538:
URL: https://github.com/apache/orc/pull/538#issuecomment-675091039


   cc @omalley 


----------------------------------------------------------------
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] [orc] findepi commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
findepi commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r470427638



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       Quoting https://en.wikipedia.org/wiki/Gregorian_calendar:
   
   _Thursday 4 October 1582 was followed by Friday 15 October 1582_
   
   >  It sounds like Java changes it in October 1581, doesn't it? 
   
   I think Java's java.util.Calendar and java.util.Date date calculations follow the Oct 4 → Oct 15 shift, do they not?




----------------------------------------------------------------
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] [orc] dongjoon-hyun commented on pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #538:
URL: https://github.com/apache/orc/pull/538#issuecomment-678052939


   Thank you, @omalley . +1 for @omalley 's suggestion.


----------------------------------------------------------------
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] [orc] dongjoon-hyun commented on a change in pull request #538: Fix Julian/Gregorian cutoff in a comment

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #538:
URL: https://github.com/apache/orc/pull/538#discussion_r471672045



##########
File path: proto/orc_proto.proto
##########
@@ -343,7 +343,7 @@ message Encryption {
 enum CalendarKind {
   UNKNOWN_CALENDAR = 0;
    // The Java default calendar changes from Julian to Gregorian
-   // in 1583.
+   // in October 1582.

Review comment:
       @findepi 
   We had better be specific to Java version because Apache Orc still supports Java 7. I'm not aware of any change in Java 7. Do you have any reference saying Java 7 changes it's default calendar or time-related stuff? IIRC, everything started at Java 8.
   - https://github.com/apache/orc/blame/master/README.md#L47




----------------------------------------------------------------
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