You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by GitBox <gi...@apache.org> on 2021/09/17 12:19:58 UTC

[GitHub] [avro] martin-g opened a new pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

martin-g opened a new pull request #1340:
URL: https://github.com/apache/avro/pull/1340


   ### Jira
   
   - [ ] My PR addresses the following [AVRO-3197](https://issues.apache.org/jira/browse/AVRO-3197)
   
   ### Tests
   
   - [ ] TBD
   
   ### Commits
   
   - [X] My commits all reference Jira issues in their subject lines.
   
   ### Documentation
   
   - [X] TBD
   


-- 
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: dev-unsubscribe@avro.apache.org

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



[GitHub] [avro] martin-g commented on a change in pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #1340:
URL: https://github.com/apache/avro/pull/1340#discussion_r713759662



##########
File path: lang/rust/tests/schema.rs
##########
@@ -19,6 +19,13 @@
 use avro_rs::{schema::Name, Error, Schema};
 use lazy_static::lazy_static;
 
+fn init() {

Review comment:
       I was also wondering about the name when introducing it, so I am almost convinced to rename it! :-)
   Thanks for the review!




-- 
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: issues-unsubscribe@avro.apache.org

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



[GitHub] [avro] martin-g commented on a change in pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #1340:
URL: https://github.com/apache/avro/pull/1340#discussion_r713742245



##########
File path: lang/rust/tests/schema.rs
##########
@@ -19,6 +19,13 @@
 use avro_rs::{schema::Name, Error, Schema};
 use lazy_static::lazy_static;
 
+fn init() {

Review comment:
       The idea is that it might be used for other needs later.
   Too bad Rust Testing does not provide something like `before()/after()`, `beforeAll()/afterAll()`.
   Using `ctor` crate looks OK-ish (https://stackoverflow.com/questions/58006033/how-to-run-setup-code-before-any-tests-run-in-rust) but it is one more dev-dependency and it helps only for the `before()` need.




-- 
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: issues-unsubscribe@avro.apache.org

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



[GitHub] [avro] martin-g merged pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
martin-g merged pull request #1340:
URL: https://github.com/apache/avro/pull/1340


   


-- 
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: dev-unsubscribe@avro.apache.org

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



[GitHub] [avro] martin-g commented on pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #1340:
URL: https://github.com/apache/avro/pull/1340#issuecomment-927843947


   @iemejia Could you please review this PR ? Thank you!


-- 
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: issues-unsubscribe@avro.apache.org

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



[GitHub] [avro] gudjonragnar commented on a change in pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
gudjonragnar commented on a change in pull request #1340:
URL: https://github.com/apache/avro/pull/1340#discussion_r713732582



##########
File path: lang/rust/tests/schema.rs
##########
@@ -19,6 +19,13 @@
 use avro_rs::{schema::Name, Error, Schema};
 use lazy_static::lazy_static;
 
+fn init() {

Review comment:
       `init` is quite general. It might be a bit clearer if this is named `init_logging` or something similar to that.




-- 
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: issues-unsubscribe@avro.apache.org

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



[GitHub] [avro] martin-g commented on pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
martin-g commented on pull request #1340:
URL: https://github.com/apache/avro/pull/1340#issuecomment-924003073


   The PR is ready for review!


-- 
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: issues-unsubscribe@avro.apache.org

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



[GitHub] [avro] gudjonragnar commented on a change in pull request #1340: AVRO-3197 Fallback to the 'type' when the logical type does not support the type

Posted by GitBox <gi...@apache.org>.
gudjonragnar commented on a change in pull request #1340:
URL: https://github.com/apache/avro/pull/1340#discussion_r713747331



##########
File path: lang/rust/tests/schema.rs
##########
@@ -19,6 +19,13 @@
 use avro_rs::{schema::Name, Error, Schema};
 use lazy_static::lazy_static;
 
+fn init() {

Review comment:
       ye I agree, or even something like pytests fixtures. 
   I see your point with possibly adding more to the init, however I often favour separating the `inits` unless they are necessary inits for all of the tests. In this case it is more of a nitpick comment, so I don't mind you disregarding it.




-- 
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: issues-unsubscribe@avro.apache.org

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