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.

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

Hotfix/v2.5.5 #283

Merged
merged 7 commits into from
Nov 20, 2024
Merged

Hotfix/v2.5.5 #283

merged 7 commits into from
Nov 20, 2024

Conversation

mkouzel-yext
Copy link
Contributor

Changes the endpoint for autocompletes to the cached version

This change updated the LiveAPI endpoints that search-core provides
so that it provides the cached version of the endpoints. Using the
cached endpoints helps improve performance.

J=WAT-4442
TEST=manual

Built test site with new change, saw that the cached endpoints
were being used.
Updates provideEndpointsTest to expect the cached version of the
LiveAPI endpoints now that we've updated search-core to supply
the cached endpoints rather than the regular ones.

J=WAT-4442
TEST=auto

provideEndpointsTest.ts passes
This commit revises the endpoint domains provided by LiveAPI so that
we supply the cached endpoints for autocompletes and normal endpoints
elsewhere.

J=WAT-4442
TEST=manual, auto

Tests expected to pass pass.
Spun up test-site manually, saw expected endpoints.
Fixes provideEndpointsTest so it only expects cached endpoints
for autocompletes.

J=WAT-4442
TEST=auto

Test passes
@mkouzel-yext mkouzel-yext requested a review from a team as a code owner November 12, 2024 21:14
k-gerner
k-gerner previously approved these changes Nov 12, 2024
@Fondryext Fondryext requested a review from a team November 12, 2024 21:16
@coveralls
Copy link

coveralls commented Nov 12, 2024

Pull Request Test Coverage Report for Build 11864104902

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 89.252%

Totals Coverage Status
Change from base Build 10814089699: 0.2%
Covered Lines: 216
Relevant Lines: 236

💛 - Coveralls

Copy link
Contributor

@Fondryext Fondryext left a comment

Choose a reason for hiding this comment

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

Two thoughts - one, I would expect package-lock.json to have also been updated. Could you make sure npm i is run, so that file is updated as well?

Second, when it's time to merge in to master we're usually a little more standardized with our commit messages. Here is an example: #278 . Note both title and commit message.

@mkouzel-yext mkouzel-yext merged commit 2050135 into master Nov 20, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants