nchammas commented on PR #57010: URL: https://github.com/apache/spark/pull/57010#issuecomment-4915650954
Part of what prompted the change to how the skip flags are handled is a combination of two factors: 1. The docs build script interface is informal. 2. In other places we are setting `SKIP_` flags to `true`, not `1`, and testing for any non-empty value. Here are some examples: https://github.com/apache/spark/blob/10e2aed582fb6c3b2aadf9ee260c3f16f35ecf5f/dev/run-tests.py#L307 https://github.com/apache/spark/blob/10e2aed582fb6c3b2aadf9ee260c3f16f35ecf5f/dev/run-tests.py#L653 https://github.com/apache/spark/blob/00fef6941f3f3958284aed9b50ad5d2798f3f997/dev/merge_spark_pr.py#L90-L92 This change does not break any existing users of the docs build script. Anyone setting `SKIP_{something}=1` and calling `jekyll` will get the same behavior as before. In a follow-up PR, I would like to adjust the other uses of `SKIP_` flags so that we have a consistent convention of handling them as "truthy" values. i.e. Any non-empty value is interpreted as true. https://github.com/apache/spark/blob/10e2aed582fb6c3b2aadf9ee260c3f16f35ecf5f/dev/run-tests.py#L647 https://github.com/apache/spark/blob/10e2aed582fb6c3b2aadf9ee260c3f16f35ecf5f/dev/run-tests.py#L657 I think having a consistent convention for how we use `SKIP_` flags will make the build infra a bit easier to understand. Does my motivation make more sense now? I can move d61ae41 into a new PR if you prefer. -- 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]
