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

Data insights in sitemap and site nav #3272

Merged
merged 3 commits into from
Mar 6, 2024
Merged

Conversation

ikesau
Copy link
Member

@ikesau ikesau commented Feb 29, 2024

This PR adds data insights to the sitemap and nav, and half-heartedly updates some data insight DB code to use Knex, before stopping arbitrarily to avoid spilling out into having to refactor the entire Gdocs codebase.

Local sitemap with DATA_INSIGHTS_INDEX_PAGE_SIZE temporarily set to 2:

data insights sitemap

The site nav Resources menu:
image

Copy link
Member

@mlbrgl mlbrgl left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -698,7 +701,10 @@ export class SiteBaker {

private async bakeDataInsights() {
Copy link
Member

Choose a reason for hiding this comment

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

you could pass knex here from the parent caller (it's already available there)

@edomt edomt merged commit cdfe2f4 into master Mar 6, 2024
24 checks passed
@edomt edomt deleted the data-insights-in-sitemap branch March 6, 2024 08:10
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.

3 participants