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

MWPW-147512 Markdown Downloader #40

Merged
merged 2 commits into from
May 8, 2024
Merged

Conversation

Brandon32
Copy link
Collaborator

@Brandon32 Brandon32 commented May 7, 2024

  • Script for downloading markdown

Resolves: MWPW-147512

Comment on lines 123 to 127
// eslint-disable-next-line arrow-body-style
const stagedUrls = list.map((entry) => {
const entryPath = entryToPath(entry);
return [entryPath, localizeStageUrl(siteURL, entryPath, stagePath, locales)];
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see why eslint would flag this. Do we need to take a look at the eslint rules?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can remove this eslint comment since I have the const.
It's two rules that that sometimes conflict, an arrow function with just a return should be on one like but a line can't be longer than 100 characters.

Comment on lines +124 to +127
const stagedUrls = list.map((entry) => {
const entryPath = entryToPath(entry);
return [entryPath, localizeStageUrl(siteURL, entryPath, stagePath, locales)];
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const stagedUrls = list.map((entry) => {
const entryPath = entryToPath(entry);
return [entryPath, localizeStageUrl(siteURL, entryPath, stagePath, locales)];
});
const stagedUrls = list.map((entry) => [entryToPath(entry), localizeStageUrl(siteURL, entryPath, stagePath, locales)]);

I imagine this may set off the linter via length though?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, and entryToPath(entry) would be used twice.

Suggested change
const stagedUrls = list.map((entry) => {
const entryPath = entryToPath(entry);
return [entryPath, localizeStageUrl(siteURL, entryPath, stagePath, locales)];
});
const stagedUrls = list.map((entry) =>[entryToPath(entry), localizeStageUrl(siteURL, entryToPath(entry), stagePath, locales)]);

return text;
} catch (e) {
if (e instanceof AbortError) {
console.warn('Fetch timed out after 1s');
Copy link
Collaborator

Choose a reason for hiding this comment

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

should update to Fetch timed out after 5s? due to signal length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should be 5s or 5000ms, I'll set the timeout to a constant and use that here and the above console log.

@Brandon32 Brandon32 merged commit 724ad9b into main May 8, 2024
4 checks passed
@Brandon32 Brandon32 deleted the bmarshal/download-script branch May 8, 2024 16:32
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