aglinxinyuan commented on PR #4416: URL: https://github.com/apache/texera/pull/4416#issuecomment-4294434901
> Ok thanks for the investigation. This is a perfect reason for > > 1. smaller PR. [Remove the redundant dataset endpoints and related dependencies #3314](https://github.com/apache/texera/pull/3314) is huge to review. > 2. careful review. I think reviewers just stamped right away, maybe because it is a huge PR. > 3. Test cases. We only noticed the issue after 1 year, not until user reports that. Test cases should have caught it much much earlier. > > @chenlica I think we need to improve on those three aspects. > > @seongjinyoon for this PR, can you start to add test cases, so that we don't accidentally delete the endpoint in the future? I believe only point 3 applies in this case. As the reviewer of #3314, I’ve taken another look at the PR: 1. It isn’t excessively large. 2. It was properly reviewed rather than just rubber-stamped. Even if I were reviewing it again today, I would still approve it. The PR removes dataset endpoints, and the changes accurately reflect the title and description. -- 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]
