Skip to content

Website: Minor scripts/build-static-content.js updates#6974

Merged
mikermcneil merged 6 commits intomainfrom
website-build-static-content-update
Aug 2, 2022
Merged

Website: Minor scripts/build-static-content.js updates#6974
mikermcneil merged 6 commits intomainfrom
website-build-static-content-update

Conversation

@eashaw
Copy link
Copy Markdown
Contributor

@eashaw eashaw commented Jul 29, 2022

@eashaw eashaw requested a review from mikermcneil July 29, 2022 18:00
@mikermcneil mikermcneil temporarily deployed to Docker Hub August 2, 2022 02:55 Inactive
@mikermcneil mikermcneil temporarily deployed to Docker Hub August 2, 2022 03:00 Inactive
@mikermcneil
Copy link
Copy Markdown
Member

@eashaw I added a bit more exposition here to further help our future selves: 60ed83c

Copy link
Copy Markdown
Member

@mikermcneil mikermcneil left a comment

Choose a reason for hiding this comment

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

Everything LGTM except what I already changed and this minor tweak

Comment thread website/scripts/build-static-content.js Outdated
throw new Error('Failed compiling markdown content: Files as currently named would result in colliding (duplicate) URLs for the website. To resolve, rename the pages whose names are too similar. Duplicate detected: ' + rootRelativeUrlPath);
}//•
rootRelativeUrlPathsSeen.push(rootRelativeUrlPath);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd move this up slightly to put it before where we generate the htmlId, so that that the code about deciding the htmlId can stay right next to the code that determines the HTML output path and writes it.

@mikermcneil mikermcneil temporarily deployed to Docker Hub August 2, 2022 03:14 Inactive
Copy link
Copy Markdown
Member

@mikermcneil mikermcneil left a comment

Choose a reason for hiding this comment

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

I went ahead and made the change. As long as that passes lint, then this looks good to merge to me.

@mikermcneil mikermcneil temporarily deployed to Docker Hub August 2, 2022 03:15 Inactive
@mikermcneil mikermcneil changed the title Website: build-static-content updates Website: Minor scripts/build-static-content.js updates Aug 2, 2022
@mikermcneil mikermcneil merged commit 9e2b7c5 into main Aug 2, 2022
@mikermcneil mikermcneil deleted the website-build-static-content-update branch August 2, 2022 03:23
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