-
Notifications
You must be signed in to change notification settings - Fork 347
Update branches for 2.13.0 release #2316
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
Conversation
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.
LGTM!
The build error relates to a missing file:
09:31:39 INFO:build_docs:asciidoctor: ERROR: index.asciidoc: line 8: include file not found: /tmp/docsbuild/Q0aeS3yX89/cloud/docs/ece-version.asciidoc
Am I right thinking that this file gets generated dynamically as part of the ECE release? We'll have to find some way to make it available for docs builds.
IIRC, that's the case. The Cloud writers have been creating an empty file, that isn't saved, so that we can get the builds to pass. |
@obierlaire confirmed that this error is expected and we can just merge/get someone to help us merge when it's time. |
@obierlaire @kellyemurphy are you saying that the file will be committed to the cloud repo as part of the release? We don't run any of the cloud build steps as part of the docs build, so unless the file already exists when the doc build starts, it will fail after this is merged. |
@gtback I believe @alpar-t introduced a change to the ECE docs that uses a generated ece-version.asciidoc file. This change breaks our Alpár, you had added this ece-version.asciidoc file to the .gitignore, but is there a way we can modify your implementation to avoid these build breaks? |
@nrichers The purpose of the change was to make it so we keep a single record of truth about the version across the repo. We do have a pattern where we do commit other generated files into the repo and check that they match what we expect, but I'm not sure it's warranted in this case. Could we just run the one Gradle target |
@alpar-t let's move the discussion for how to best address this into the related issue in the cloud repo?
EDIT EDIT: We ended up creating a workaround in the cloud repo to get the Elastic docs build to pass. The CI checks in this case are likely right, so we don't want to force merge the PR after all. |
@elasticmachine run elasticsearch-ci/docs |
@elasticmachine run elasticsearch-ci/docs |
Good news, bad news scenario: on the upside, the ECE 2.13 docs build works, thanks to the workaround mentioned above. On the downside, we now have a bunch of broken links:
I will work on fixing these. |
@elasticmachine run elasticsearch-ci/docs |
Updates the branches for: ECE, ecctl for the
upcoming ECE 2.13.0 release.