InigoSJ commented on pull request #12074: URL: https://github.com/apache/beam/pull/12074#issuecomment-648932828
@pabloem Some questions that I would need help with: 1 - Is the name `CombinerWithoutDefaults` good enough for the parent class? Am I adding it in the right place? 2 - All my tests passed, but just to double check, am I missing any `*args, **kwargs` anywhere? 3 - I needed to add `timestamp = 0` in many tests, should I do it as a global variable? maybe just use 0 without referencing `timestamp`? 4 - Assert at *795*: I used a bigger window (180) so that there's 2 outputs. Should I keep it as the previous assert that doesn't have windows (i.e., with one output element)? 5 - Checking the file, I don't see any pydoc that's needed, but maybe you think I should add something Thanks a lot for your help! ---------------------------------------------------------------- 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: [email protected]
