Skip to content

Use fetch to get images instead of axios (BL-14629) #119

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

Merged
merged 1 commit into from
Apr 22, 2025

Conversation

andrew-polk
Copy link
Contributor

@andrew-polk andrew-polk commented Apr 17, 2025

This change is Reviewable

Copy link

stackblitz bot commented Apr 17, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link
Member

@StephenMcConnel StephenMcConnel left a comment

Choose a reason for hiding this comment

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

One question, but otherwise looks good to me.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @andrew-polk)


src/images.ts line 155 at r1 (raw file):

  // so we switched to fetch. Just a guess, but probably imgur keeps locking down
  // what it suspects as code running to scrape images. Apparently, imgur is getting
  // to be more and more of a liability, so we should probably stop using it.

Is the "particular gif" fetch no longer a problem or the "particular gif" no longer needed? Or is this fetch method different from the node-fetch mentioned in the comment?

Copy link
Contributor Author

@andrew-polk andrew-polk left a comment

Choose a reason for hiding this comment

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

Reviewable status: 2 of 3 files reviewed, 1 unresolved discussion (waiting on @StephenMcConnel)


src/images.ts line 155 at r1 (raw file):

Previously, StephenMcConnel (Steve McConnel) wrote…

Is the "particular gif" fetch no longer a problem or the "particular gif" no longer needed? Or is this fetch method different from the node-fetch mentioned in the comment?

Great question. They're different. I enhanced the comment.

Copy link
Contributor Author

@andrew-polk andrew-polk left a comment

Choose a reason for hiding this comment

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

Dismissed @StephenMcConnel from a discussion.
Reviewable status: 2 of 3 files reviewed, all discussions resolved (waiting on @StephenMcConnel)

@andrew-polk andrew-polk merged commit dde701e into main Apr 22, 2025
1 of 2 checks passed
@andrew-polk andrew-polk deleted the BL14629_Imgur429 branch April 22, 2025 15:41
Copy link

🎉 This PR is included in version 0.16.2-alpha.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants