You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "nchammas (via GitHub)" <gi...@apache.org> on 2024/01/08 22:05:20 UTC

[PR] [SPARK-46437][DOCS] Add custom tag for conditional Jekyll includes [spark]

nchammas opened a new pull request, #44630:
URL: https://github.com/apache/spark/pull/44630

   ### What changes were proposed in this pull request?
   
   Add a [custom Jekyll tag][custom] to enable us to conditionally include files in our documentation build in a more user-friendly manner. [This example][example] demonstrates how a custom tag can build on one of Jekyll's built-in tags.
   
   [custom]: https://github.com/Shopify/liquid/wiki/Liquid-for-Programmers#create-your-own-tags
   [example]: https://github.com/Shopify/liquid/issues/370#issuecomment-688782101
   
   This greatly simplifies the Liquid directives required to include files.
   
   Without this change, files have to be included as follows:
   
   ```
   {% for static_file in site.static_files %}
       {% if static_file.name == 'generated-agg-funcs-table.html' %}
           {% include_relative generated-agg-funcs-table.html %}
           {% break %}
       {% endif %}
   {% endfor %}
   ```
   
   With this change, they can be included more intuitively:
   
   ```
   {% include_relative_if_exists generated-agg-funcs-table.html %}
   ```
   
   If this file is missing, the rendered output will show the following text instead of failing the build:
   
   ```
   placeholder for missing file: generated-agg-funcs-table.html
   ```
   
   ### Why are the changes needed?
   
   Jekyll does not have a succinct way to [check if a file exists][check], so the required directive is very cumbersome.
   
   We need the ability to do this so that we can [build the docs successfully with `SKIP_API=1`][build], since many includes reference files that are only generated when `SKIP_API` is _not_ set.
   
   [check]: https://github.com/jekyll/jekyll/issues/7528
   [build]: https://github.com/apache/spark/pull/44627
   
   ### Does this PR introduce _any_ user-facing change?
   
   No.
   
   ### How was this patch tested?
   
   Manually building and reviewing the docs.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   No.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46437][DOCS] Add custom tags for conditional Jekyll includes [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #44630:
URL: https://github.com/apache/spark/pull/44630#issuecomment-1884012877

   Merged to master.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46437][DOCS] Add custom tags for conditional Jekyll includes [spark]

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #44630: [SPARK-46437][DOCS] Add custom tags for conditional Jekyll includes
URL: https://github.com/apache/spark/pull/44630


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46437][DOCS] Add custom tags for conditional Jekyll includes [spark]

Posted by "nchammas (via GitHub)" <gi...@apache.org>.
nchammas commented on PR #44630:
URL: https://github.com/apache/spark/pull/44630#issuecomment-1883720370

   Documentation build [passing here][2].
   
   [2]: https://github.com/nchammas/spark/actions/runs/7465967638/job/20316346740
   
   cc @HyukjinKwon - These new Jekyll tags properly solve the problem [described here][1].
   
   I will also be using them extensively if/when I get committer buy-in on #44300.
   
   [1]: https://github.com/apache/spark/pull/44393#discussion_r1445378532


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org