andygrove commented on PR #23893: URL: https://github.com/apache/datafusion/pull/23893#issuecomment-5206580137
On the hardcoded-path fragility from the approval: the part that worried me most is the one you named, that there is no guarantee it does not silently fail. A `grep` over a directory that no longer exists and a `grep` that genuinely found nothing produce the same empty output, so a moved `FunctionRegistry.scala` turns into "Spark does not test this" and the audit reads clean. Rather than a second skill to audit this one, I added a section up front making an unresolved path a stop condition: name it in the report, search for where it moved by content instead of by path, and if it cannot be found, mark every conclusion that depended on that step as unestablished. The grep patterns are more durable than the directories they point at, so the recovery path is usually short. Related, @comphead asked whether Comet and Sail needed local clones. Step 4 previously greased over a missing Comet clone with `2>/dev/null`, which is exactly this failure mode: no clone read as no divergences. Both are now searched over `gh api search/code`, no clone required, with the local grep as a faster fallback when a checkout happens to be there, and an instruction to report Step 4 as not performed if the API call fails. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
