You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/12/05 10:09:02 UTC
[GitHub] [arrow] AlenkaF opened a new pull request, #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
AlenkaF opened a new pull request, #14839:
URL: https://github.com/apache/arrow/pull/14839
This PR is a complementary work to https://github.com/apache/arrow-site/pull/275.
It adds a version warning script to the `docs/source/_templates/layout.html` and changes the stable version value in _versionwarnings.js_ with adding a line to `dev/release/post-08-docs.sh`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1043200290
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
I can change the name of the file and move it to `/docs/dev/_static/versionwarning.js`. As Joris mentioned, changes in the arrow-site repo will be needed afterwards.
Will wait a day or two to see if there is any other opinion/change of the name or file location and make the changes then.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14839:
URL: https://github.com/apache/arrow/pull/14839#issuecomment-1337072635
https://issues.apache.org/jira/browse/ARROW-18363
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1040681137
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
The question we have to answer here is whether we want to maintain this `versionwarning.js` file in the arrow repo or in the arrow-site repo.
Because if we put it in `_static`, that folder is created by the output of sphinx, and so then we should include `versionwarning.js` in the sphinx source here in the arrow repo. On the other hand, if the goal is that it is a single file that supports all the different versions we currently include in the /docs in arrow-site, we can also maintain it in arrow-site (and if we maintain it there, I think it is easier to to put it in /docs instead of /docs/_static). This is the approach that mne-python and scikit-learn (on which the current versionwarning.js file is based) are using.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1045590844
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
> Can we use Sphinx variable for it something like the following?
Not sure, can try 👍
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1041459752
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
In my opinion, I don't want to maintain files in `apache/arrow-site`'s `asf-site` branch as much as possible.
The branch is sometimes updated automatically. It may delete some files unexpectedly. See also: https://github.com/apache/arrow-site/pull/151
The branch has many large commits. It's difficult to follow history of maintained files.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1039938307
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
There is a separate PR that adds the `versionwarnings.js` file: https://github.com/apache/arrow-site/pull/275 .
It has to be merged first.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1040526832
##########
dev/release/post-08-docs.sh:
##########
@@ -83,6 +83,8 @@ git checkout docs/c_glib/index.html
if [ "$is_major_release" = "yes" ] ; then
mv docs_temp docs/${previous_version}
fi
+# Correct the stable version in versionwarnings.js
+sed -i -e "s/${previous_version}/${version}/g" docs/versionwarning.js
Review Comment:
Thank you Kou for all of the suggestions! Will make changes to the PR today.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #14839:
URL: https://github.com/apache/arrow/pull/14839#issuecomment-1337585018
For the record, this is how this looks in the Python docs:
https://docs.python.org/3.6/library/ftplib.html
This is the corresponding template code:
https://github.com/python/cpython/blob/main/Doc/tools/templates/layout.html#L3-L11
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1041913186
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Ah, yes, that's a good point. I agree that we should avoid having the source only in the asf-site branch. I suppose that in theory we could still have the file in the arrow-site repo in the master branch and ensure this gets put in the docs/ repo on deployment. But that's also some additional complexity, so I suppose then we can just as well leave it in the arrow repo (where at least it lives in the docs/ repo).
We will just have to remember that whenever we want to do an update to the versionwarning.js file, we also have to port that patch to arrow-site to actually start using it (otherwise it only gets used once there is a new release)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1041932637
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
> We will just have to remember that whenever we want to do an update to the versionwarning.js file, we also have to port that patch to arrow-site to actually start using it (otherwise it only gets used once there is a new release)
I can add a warning comment in the `versionwarning.js` if that would 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1040528546
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Will move the JS file to `docs/source/_static/` as there seems to be a consensus on this topic.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1043093152
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
> How about referring `/docs/dev/_static/versionwarning.js` instead of `/docs/_static/versionwarning.js`?
Ah, sorry, I just merged the PR on the site repo before checking the latest comments here. Now, if we decide to go with /dev/, then it should be easy to still update that in the arrow-site repo.
While doing it in /dev/ would avoid two PRs, in practice you can't test a change in this repo though, and you always have to test it in arrow-site as well to check the changes. So then actually doing a PR there as well isn't too bad.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1053310579
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Sounds good to me!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1039940259
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
And the link to the file: https://github.com/AlenkaF/arrow-site/blob/73fb93f1f2254efd11b83e8de8b020d085d009e7/docs/versionwarning.js
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1043850092
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Can we use Sphinx variable for it something like the following?
```diff
diff --git a/ci/scripts/python_build.sh b/ci/scripts/python_build.sh
index cfac68bd6e..d02d654205 100755
--- a/ci/scripts/python_build.sh
+++ b/ci/scripts/python_build.sh
@@ -83,5 +83,10 @@ popd
if [ "${BUILD_DOCS_PYTHON}" == "ON" ]; then
ncpus=$(python -c "import os; print(os.cpu_count())")
- sphinx-build -b html -j ${ncpus} ${arrow_dir}/docs/source ${build_dir}/docs
+ sphinx-build \
+ -b html \
+ -j ${ncpus} \
+ -D version_warning_js=://arrow.apache.org/docs/dev/_static/version_warning.js \
+ ${arrow_dir}/docs/source \
+ ${build_dir}/docs
fi
diff --git a/docs/source/_templates/layout.html b/docs/source/_templates/layout.html
index 50511b1416..fd5186fdac 100644
--- a/docs/source/_templates/layout.html
+++ b/docs/source/_templates/layout.html
@@ -30,5 +30,5 @@
{# Add version warnings #}
{% block footer %}
{{ super() }}
- <script type="text/javascript" src="/docs/versionwarning.js"></script>
+ <script type="text/javascript" src="{{ pathto(version_warning_js, 1) }}"></script>
{% endblock %}
diff --git a/docs/source/conf.py b/docs/source/conf.py
index 2c3686df87..92b7f2cbf4 100644
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -148,6 +148,9 @@ autosummary_generate = True
# The master toctree document.
master_doc = 'index'
+# Path to version_warning.js.
+version_warning_js = "_static/version_warning.js"
+
# General information about the project.
project = u'Apache Arrow'
copyright = f'2016-{datetime.datetime.now().year} Apache Software Foundation'
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1045590844
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
> Can we use Sphinx variable for it something like the following?
Sure, can try 👍
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1039946521
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
> Why can't the file be added in `docs/source/_static` instead?
That is something Joris also suggested, but at the end decided it could stay as is. I do not mind changing, if it is preferred. cc @jorisvandenbossche
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1042688896
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
BTW, I like `version-warnings.js` or `version_warnings.js` (I want word separator for readability)...
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1053749399
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
I'm OK with it too.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14839:
URL: https://github.com/apache/arrow/pull/14839#issuecomment-1364106123
Benchmark runs are scheduled for baseline = 387e95ad575fd158bb2758e97800716d3976fce2 and contender = 305026f62c172acfe0ed6549288e209358247dda. 305026f62c172acfe0ed6549288e209358247dda is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/fb2f4825619d4309a5c57620981c6108...537c45c51c394a79a2866d3feef0823a/)
[Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/5f7e798a802b46eb9935ce2ee52d45d8...8ab94d6440774195ad5e7ea96ab43967/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/1ee28cf44e1a443bb1f6ffb699d5c156...a47ca4c7cdf4445492752c41ac7b7402/)
[Finished :arrow_down:0.1% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/d5c886b4beb04dc2b8aa5dc5cbf7f3d4...2cc5039ebc6c4037a9cf470a28a36793/)
Buildkite builds:
[Finished] [`305026f6` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2060)
[Failed] [`305026f6` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2082)
[Finished] [`305026f6` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2054)
[Finished] [`305026f6` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2074)
[Finished] [`387e95ad` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2059)
[Failed] [`387e95ad` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2081)
[Finished] [`387e95ad` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2053)
[Finished] [`387e95ad` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2073)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1050436231
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Yes. I wanted to solve "in practice you can't test a change in this repo though, and you always have to test it in arrow-site as well to check the changes".
I thought that we can test almost features (I understand that we can't test all features) with this approach. We don't need to change `src=.../versionwaning.js` in all `.html` manually to test locally with this approach.
If this approach doesn't help `versionwarning.js` developers/maintainers, I'm OK without this approach. (I'm OK that they always need apache/arrow-site.)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1053754432
##########
docs/source/_static/versionwarnings.js:
##########
@@ -0,0 +1,134 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
It seems that `s` is included in file name.
```diff
-versionwarnings.js
+versionwarning.js
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1053995994
##########
docs/source/_static/versionwarnings.js:
##########
@@ -0,0 +1,134 @@
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Oh sorry for that, great catch. I corrected the file name and the src path.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1039939791
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Why can't the file be added in `docs/source/_static` instead?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1041963321
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Yes, that sounds good
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1047181595
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
@kou what would be the goal of that variable? So that you can build the docs locally with a different path (one that would match the local path)?
That still won't help that you can test this locally when building the arrow docs, though, as it the javascript code needs the structure of different versions in subdirectories as we have in that arrow-site branch.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1042687991
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
How about referring `/docs/dev/_static/versionwarning.js` instead of `/docs/_static/versionwarning.js`?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1053249653
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
I propose to keep the current path to the JS file (docs/_static) and also the current way of injecting the src link in the html files. This way there will be no further changes needed in the arrow-site repo.
If we decide to move the file to the dev/ folder or use a variable in html (in my short attempt I wasn't able to make Sphinx work with variables) I think it is better to create a follow-up issue.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorisvandenbossche merged pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
jorisvandenbossche merged PR #14839:
URL: https://github.com/apache/arrow/pull/14839
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] AlenkaF commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
AlenkaF commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1040817194
##########
dev/release/post-08-docs.sh:
##########
@@ -83,6 +83,8 @@ git checkout docs/c_glib/index.html
if [ "$is_major_release" = "yes" ] ; then
mv docs_temp docs/${previous_version}
fi
+# Correct the stable version in versionwarnings.js
+sed -i -e "s/${previous_version}/${version}/g" docs/versionwarning.js
Review Comment:
Changes to the JS file (using `/docs/_static/versions.json` to get the stable version) can be found here:
https://github.com/apache/arrow-site/pull/275/commits/7fab4a9ca3751d45da0861279c558e79e5554667
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1039749893
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
Is this PR missing the file `/docs/versionwarning.js`?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on a diff in pull request #14839: ARROW-18363: [Docs] Include warning when viewing old docs (redirecting to stable/dev docs)
Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #14839:
URL: https://github.com/apache/arrow/pull/14839#discussion_r1040092560
##########
dev/release/post-08-docs.sh:
##########
@@ -83,6 +83,8 @@ git checkout docs/c_glib/index.html
if [ "$is_major_release" = "yes" ] ; then
mv docs_temp docs/${previous_version}
fi
+# Correct the stable version in versionwarnings.js
+sed -i -e "s/${previous_version}/${version}/g" docs/versionwarning.js
Review Comment:
BTW, we have https://arrow.apache.org/docs/_static/versions.json . Can we use it instead of embedding the latest stable version into https://github.com/AlenkaF/arrow-site/blob/73fb93f1f2254efd11b83e8de8b020d085d009e7/docs/versionwarning.js#L4 ?
##########
dev/release/post-08-docs.sh:
##########
@@ -83,6 +83,8 @@ git checkout docs/c_glib/index.html
if [ "$is_major_release" = "yes" ] ; then
mv docs_temp docs/${previous_version}
fi
+# Correct the stable version in versionwarnings.js
+sed -i -e "s/${previous_version}/${version}/g" docs/versionwarning.js
Review Comment:
`sed -i` isn't portable. It works with GNU sed but not BSD sed that is shipped on macOS.
Could you use `sed -i.bak ...` and `rm versionwarning.js.bak` like `dev/release/utils-prepare.sh` instead?
##########
docs/source/_templates/layout.html:
##########
@@ -26,3 +26,9 @@
{# Silence the navbar #}
{% block docs_navbar %}
{% endblock %}
+
+{# Add version warnings #}
+{% block footer %}
+ {{ super() }}
+ <script type="text/javascript" src="/docs/versionwarning.js"></script>
Review Comment:
I like `docs/source/_static/` because https://github.com/apache/arrow/blob/master/docs/source/_static/versions.json uses `docs/source/_static/`.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org