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

feat(gatsby-core-utils): Add file download functions #29531

Merged
merged 7 commits into from
Feb 20, 2021
Merged

feat(gatsby-core-utils): Add file download functions #29531

merged 7 commits into from
Feb 20, 2021

Conversation

ascorbic
Copy link
Contributor

Move the file download function fetchRemoteNode to gatsby-core-utils and rename it to fetchRemoteFile

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 16, 2021
@ascorbic ascorbic force-pushed the chore/move-file-functions branch from 32d4d43 to bea141d Compare February 16, 2021 10:16
@ascorbic ascorbic added topic: media Related to gatsby-plugin-image, or general image/media processing topics topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer topic: media Related to gatsby-plugin-image, or general image/media processing topics labels Feb 16, 2021
@ascorbic ascorbic marked this pull request as ready for review February 16, 2021 11:02
@ascorbic ascorbic requested a review from wardpeet February 17, 2021 12:19
laurieontech
laurieontech reviewed Feb 17, 2021
View reviewed changes
Copy link
Contributor

@laurieontech laurieontech left a comment

Choose a reason for hiding this comment

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

Does this solve the issue entirely though? We use both remoteFileNode and createFileNode. This appears to only move the former out of gatsby-source-filesystem?

@ascorbic
Copy link
Contributor Author

The idea would be that we stop creating file nodes, and just download the file

wardpeet
wardpeet reviewed Feb 18, 2021
View reviewed changes
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

This looks good! This is going to be helpful for other plugins as well. Can you port some tests over?

@ascorbic ascorbic added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Feb 18, 2021
@ascorbic ascorbic requested a review from wardpeet February 18, 2021 13:54
@ascorbic ascorbic removed the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Feb 18, 2021
laurieontech
laurieontech approved these changes Feb 19, 2021
View reviewed changes
Copy link
Contributor

@laurieontech laurieontech left a comment

Choose a reason for hiding this comment

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

Yay tests!

@ascorbic ascorbic merged commit 1a9469d into master Feb 20, 2021
@ascorbic ascorbic deleted the chore/move-file-functions branch February 20, 2021 08:47
This was referenced Mar 12, 2021
Closed
Merged
Merged
Closed
This was referenced Mar 17, 2021
Closed
Closed
Closed
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Closed
Closed
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
@ascorbic
feat(gatsby-core-utils): Add file download functions (gatsbyjs#29531)
cc1c0cd 
* feat(gatsby-core-utils): Add file download functions

* skiplibcheck

* Export interface

* Port tests

* Fix type

Co-authored-by: gatsbybot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurieontech laurieontech laurieontech approved these changes

@wardpeet wardpeet Awaiting requested review from wardpeet

Assignees
No one assigned
Labels
topic: source-plugins Relates to the Gatsby source plugins (e.g. -filesystem)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ascorbic @wardpeet @laurieontech