jrgemignani commented on PR #807: URL: https://github.com/apache/age/pull/807#issuecomment-1507712147
@rrrokhtar Here are the problems that I have with this PR - 1. I'm not sure that factorial is a wise function to add to AGE as it easily overflows. Does Neo4j have it? 2. Your code limits its usefulness by using the wrong variable types and sizes. 3. While you have a nice graphic showing what it can do, you have zero regression tests written for it. Please justify why this is needed, or is a good idea to have. If it is justified, please fix your code to make it more mathematically useful. And write a complete set of regression tests for all usages and edge cases. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
