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

Add Algolia DocSearch as search provider #168

Merged
merged 8 commits into from
Aug 31, 2020
Merged

Add Algolia DocSearch as search provider #168

merged 8 commits into from
Aug 31, 2020

Conversation

rekmarks
Copy link
Member

@rekmarks rekmarks commented Aug 28, 2020 •
edited
Loading

This adds Algolia DocSearch as our search provider, enabling full-text search for our docs. The default VuePress search provider is an-inline JavaScript implementation that only searches Markdown headers. Algolia is used by a ton of popular JavaScript projects, including Babel, TypeScript, React, and Bootstrap.

  • Add dotenv dev dependency for Algolia DocSearch secrets
  • Create build.js for local builds
    • Uses dotenv
  • Create ci-build.sh for CI builds
    • Uses build.js
    • Sets environment variables for build script via hidden CI environment variables
  • Update file deletion command in deploy script
    • To ensure that .env-template is deleted
  • Add instructions for creating .env file in readme

@rekmarks rekmarks requested review from BboyAkers and a team as code owners August 28, 2020 03:27
@rekmarks rekmarks force-pushed the add-algolia-search branch from 7f0fe75 to e894fc7 Compare August 28, 2020 03:40
@rekmarks rekmarks requested a review from whymarrh August 28, 2020 05:58
rekmarks
rekmarks commented Aug 28, 2020
View reviewed changes
docs/.vuepress/config.js Outdated
@@ -1,4 +1,6 @@
module.exports = _ctx => ({
module.exports = (_ctx) => ({
sourceDir: 'docs',
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that the source directory is now specified in the config as opposed to an inline argument.

whymarrh
whymarrh suggested changes Aug 28, 2020
View reviewed changes
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

I left a few inline suggestions. Additionally—and I don't have a source for this so consider it a nitpick—I think that .env.dist might be a more common name for the "distributed" template version of .env. We could rename that file if that resonates with you.

.circleci/config.yml Outdated
yarn build
./ci-build.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

We could maybe simplify this by inlining the CI-specific check here:

if [[ -z "$ALGOLIA_API_KEY" || -z "$ALGOLIA_INDEX_NAME" ]]
then
  printf 'ERROR: %s\n' 'Algolia API key or index name not set.' >&2
  exit 1
fi

And we could continue to use yarn build (more below).

(Two subtle changes made to this snippet: [[ in place of [ and single quotes to avoid the escapes.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Inlined in config.yml in 2d8c946

.circleci/config.yml Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@rekmarks rekmarks requested a review from whymarrh August 28, 2020 17:15
@rekmarks rekmarks force-pushed the add-algolia-search branch from b8449b1 to 80b9815 Compare August 28, 2020 17:23
@whymarrh

This comment has been minimized.

Sign in to view
@BboyAkers
Copy link
Collaborator

BboyAkers commented Aug 29, 2020 •
edited
Loading

@rekmarks not too familiar with Algolia. I'm assuming this is going to make searching the docs quicker and more accurate? 🙂 Nice find!

@rekmarks
Copy link
Member Author

@BboyAkers even better, it'll enable full-text search! Right now we only have headers. I should add some more of my research to this PR, I'll do that tomorrow 👍

whymarrh
whymarrh approved these changes Aug 31, 2020
View reviewed changes
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM!

@rekmarks
Copy link
Member Author

Added some context in the PR description, thanks @whymarrh and @BboyAkers!

@rekmarks rekmarks merged commit b8d45b3 into master Aug 31, 2020
@rekmarks rekmarks deleted the add-algolia-search branch August 31, 2020 17:07
@rekmarks rekmarks changed the title Add Algolia DocSearch secrets to CI build script Add Algolia DocSearch to documentation Aug 31, 2020
@rekmarks rekmarks changed the title Add Algolia DocSearch to documentation Add Algolia DocSearch as search provider Aug 31, 2020
@rekmarks rekmarks mentioned this pull request Aug 31, 2020
Merged
This was referenced Oct 7, 2020
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@whymarrh whymarrh whymarrh approved these changes

@BboyAkers BboyAkers Awaiting requested review from BboyAkers

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

Successfully merging this pull request may close these issues.
3 participants
@rekmarks @whymarrh @BboyAkers