-
Notifications
You must be signed in to change notification settings - Fork 292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix readthedocs PR builds #1894
Conversation
@@ -5,7 +5,7 @@ | |||
version: 2 | |||
formats: [] | |||
build: | |||
image: latest | |||
os: ubuntu-22.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please explain why we need to have this change, to be more specific, please quote the error in the commit message, and reference the link to RtD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I've made the suggested changes.
Please let me know if I need to add something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK "image to os" I get, but why change away from "latest"? This just sets us up for a stale jump in future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmick see https://docs.readthedocs.io/en/stable/config-file/v2.html#build-os . so we only have two options: ubuntu-20.04, ubuntu-22.04
probably we could also have referenced it in the commit message, and explain that "ubuntu-22.04" is preferred, as (i guess), presumably, it is newer and hence better, like with newer tool chain and is better supported, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dmick there was some discussion about adding "latest"/"ubuntu-latest-lts" as an option in readthedocs/readthedocs.org#8861
but seems like RtD decided against it because they wanted to avoid breaking users' builds whenever RtD updated "latest" so they're letting users update on their own timeline. Their explanation: readthedocs/readthedocs.org#10401 (comment)
@tchaikov I'll make sure to mention the reason of using a version more explicitly whenever there's next update
builds are failing because support for deprecated “build.image” is fully removed by readthedocs, need to use "build.os" instead. ref: https://blog.readthedocs.com/use-build-os-config/ error: https://readthedocs.org/projects/teuthology/builds/22250705/ Signed-off-by: Vallari Agrawal <val.agl002@gmail.com>
c118f94
to
588b8e3
Compare
OK. Good reason for the change. It's good to explain every nonobvious change when submitting new code, if not for right now, for 2 years from now when someone tries to understand. Thanks for the explanation. |
I'll keep that in mind when writing new commits. Thank you. |
fixes recently failing readthedocs builds like:
https://readthedocs.org/projects/teuthology/builds/22250705/