Skip to content
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.

Sign up for GitHub

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

Jump to bottom

UI bundle previews #33

Merged
merged 2 commits into from
Oct 19, 2023
Merged

UI bundle previews #33

merged 2 commits into from
Oct 19, 2023

Conversation

mlr
Copy link
Contributor

@mlr mlr commented Oct 19, 2023 •
edited
Loading

This PR adds an Antora UI bundle preview build and a full Antora preview build that will both utilize whatever UI bundle changes exist on your Pull Request.

The reason I went with both a UI bundle preview, and a full Antora build preview is so we have the best of both worlds. Without the UI bundle preview build it's harder to make UI bundle changes that also require content changes to test properly. Without the Antora build preview you don't have the ability to see how your UI changes impact the real Antora build. With both previews we get both for added confidence in our change.

We will also receive any lint errors that happen as part of the UI bundle build or the Antora site build as comments on the PR.

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

UI bundle preview build successful! ✅
Deploying preview to GitHub Pages.
Deployment successful! View preview

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

Antora site build successful! ✅
Deploying draft to GitHub Pages.
Deployment successful! View draft

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

UI bundle preview build successful! ✅
Deploying preview to GitHub Pages.
Deployment successful! View preview

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

Antora site build successful! ✅
Deploying draft to GitHub Pages.
Deployment successful! View draft

mlr
mlr commented Oct 19, 2023
View reviewed changes
gulp.d/tasks/build-preview-pages.js
if (uiModel.env.UI_ROOT_PATH_PREFIX) {
uiRootPath = path.join('/', uiModel.env.UI_ROOT_PATH_PREFIX, uiRootPath)
}
uiModel.uiRootPath = uiRootPath
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only real deviation necessary from the default antora UI build task. In order to support having the preview build reside in a subdirectory, I needed a way to provide the path prefix at build time.

mlr
mlr commented Oct 19, 2023
View reviewed changes
src/css/ds-vars.css Outdated
@@ -1,6 +1,6 @@
:root {
--ds-common-black: #090909;
--ds-common-white: #fff;
--ds-common-white: #ff0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove this before merging, but needed a way to test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My eyes 😵

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😂

@mlr mlr force-pushed the ui-bundle-previews branch from b24d92b to ddfba58 Compare October 19, 2023 20:18
@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023

UI bundle preview build failure! ❌

[20:18:52] Using gulpfile ~/work/docs-ui/docs-ui/gulpfile.js [20:18:52] Starting 'lint'... [20:18:52] Starting 'lint:css'... [20:18:52] Starting 'lint:js'... [20:18:54] /home/runner/work/docs-ui/docs-ui/src/js/08-toc-rail.js 5:56 error Extra semicolon semi ✖ 1 problem (1 error, 0 warnings) 1 error and 0 warnings potentially fixable with the --fix option. [20:18:54] 'lint:js' errored after 1.91 s [20:18:54] �[31mESLintError�[39m in plugin "�[36mgulp-eslint�[39m" Message: Failed with 1 error [20:18:54] 'lint' errored after 1.92 s

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023

Antora site build failure! ❌

[20:19:16] Using gulpfile ~/work/datastax-docs-site/datastax-docs-site/ui-bundle-preview/gulpfile.js [20:19:16] Starting 'bundle'... [20:19:16] Starting 'clean'... [20:19:16] Finished 'clean' after 12 ms [20:19:16] Starting 'lint:css'... [20:19:16] Starting 'lint:js'... [20:19:18] /home/runner/work/datastax-docs-site/datastax-docs-site/ui-bundle-preview/src/js/08-toc-rail.js 5:56 error Extra semicolon semi ✖ 1 problem (1 error, 0 warnings) 1 error and 0 warnings potentially fixable with the --fix option. [20:19:18] 'lint:js' errored after 1.97 s [20:19:18] �[31mESLintError�[39m in plugin "�[36mgulp-eslint�[39m" Message: Failed with 1 error [20:19:18] 'bundle' errored after 1.98 s

@mlr mlr requested a review from eric-schneider October 19, 2023 20:20
@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

UI bundle preview build successful! ✅
Deploying preview to GitHub Pages.
Deployment successful! View preview

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

Antora site build successful! ✅
Deploying draft to GitHub Pages.
Deployment successful! View draft

eric-schneider
eric-schneider approved these changes Oct 19, 2023
View reviewed changes
Copy link
Contributor

@eric-schneider eric-schneider left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great as far as I can tell. Nice work!

The only thing I would say is that it would be nice if the preview and draft build notices were merged into a single comment so that the comment stream doesn't get overly polluted. But I recognize that's probably just a "nice to have" and not necessarily a blocker.

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023

Everything looks great as far as I can tell. Nice work!

The only thing I would say is that it would be nice if the preview and draft build notices were merged into a single comment so that the comment stream doesn't get overly polluted. But I recognize that's probably just a "nice to have" and not necessarily a blocker.

Agreed. I had considered this too, but due to the way the builds are happening in parallel (bundle preview happens on this repository, Antora preview happens on datastax-docs-site) it would have been overly complex to manage the sequence of the comment updates. For example, you would potentially get a success message for the bundle preview, followed by a link to the Antora preview. Separating them is a bit more noisy, but I think it eliminates confusion as to which link is for which build.

@mlr mlr force-pushed the ui-bundle-previews branch from ea9a2e4 to c604f83 Compare October 19, 2023 20:43
@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

UI bundle preview build successful! ✅
Deploying preview to GitHub Pages.
Deployment successful! View preview

@mlr
Copy link
Contributor Author

mlr commented Oct 19, 2023 •
edited
Loading

Antora site build successful! ✅
Deploying draft to GitHub Pages.
Deployment successful! View draft

@mlr mlr merged commit 29026bf into main Oct 19, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-schneider eric-schneider eric-schneider approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mlr @eric-schneider