You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@calcite.apache.org by "julianhyde (via GitHub)" <gi...@apache.org> on 2023/01/20 18:28:23 UTC

[GitHub] [calcite-avatica] julianhyde commented on pull request #205: [CALCITE-5487] Proper semantics for TIMESTAMP WITH LOCAL TIME ZONE

julianhyde commented on PR #205:
URL: https://github.com/apache/calcite-avatica/pull/205#issuecomment-1398777208

   If it's not too much trouble, could you refactor the tests as a commit *before* the fix? It's disconcerting to see lots of test changes in a bug fix; I'd prefer to just see additions. The commit message cold be 'Refactor XxxTest'
   
   I think that splitting accessor classes will help you achieve the 'hygiene' I referred to in https://issues.apache.org/jira/browse/CALCITE-5488.
   
   The commit message should be the bug summary, i.e. '[CALCITE-5487] Support TIMESTAMP WITH LOCAL TIME ZONE in ResultSet'. Leaving the jira case number off makes it more difficult for the reviewer. 


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

To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org

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