pitrou commented on pull request #9702: URL: https://github.com/apache/arrow/pull/9702#issuecomment-1016238241
> @pitrou Please review again. There are several issues we still need to discuss: > > 1. What exactly should be done to `FileVersion`? Shall I create an `options.cc` which currently doesn't exist in order to put the `FileVersion::ToString` there? Yes, you should. > 2. What are the best practices related to generating a single random integer? Surprisingly I can't find any instance of system time being used as the seed in Arrow. Furthermore seeds in the code base are often constants. I wonder how to do so without hitting the same value all the time. Test data should always be generated using a fixed seed, so that failures can be reproduced and debugged. This means that it does not make sense to generate a _single_ integer/number/etc. In this case, I don't think it makes sense to use random data at all. Just hardcode the values of interest you want to test for. > Moreover given that int64_t is by default on the stack it doesn't seem that having a vector of ints is problematic. `std::vector<int64_t>` which I currently use both makes sense conceptually and is widely used in the Arrow source code. If we use this type then the move suggestion you made can not be acted upon. See my inline response about moving. -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
