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

feat(gatsby-source-airtable): Remove whitespaces in filenames #371

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ZeldOcarina
Copy link

I want to enable whitespace removal in publicURLs.

Due to Meta new specifications, I cannot use og:images anymore if there's any whitespace in there.

Let me know if this is acceptable or I'll find a different workaround like downloading the og:images in a prebuild script and passing those in the static folder.

This was proposed in the old repo here: jbolda/gatsby-source-airtable#485 and I have been asked to post it here.

Thanks for the review!

@changeset-bot
Copy link

changeset-bot bot commented Feb 3, 2023

⚠️ No Changeset found

Latest commit: e81bf6f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ZeldOcarina
Copy link
Author

@jbolda hi! I've added the pull request here, would you be so kind to check it out? It's half-a-line of code only! :)

@moonmeister
Copy link
Contributor

moonmeister commented Feb 8, 2023

@ZeldOcarina Thanks for the PR. Couple questions

  1. If you could link to those new requirements (or quote if needed) that'd be helpful.
  2. Do you see any potential here for this being a breaking change? Any scenario where that'd be possible?
  3. Please add a changeset based on the question above. yarn cs in root will take you through that process.
  4. Feel free to run yarn ac from the root to add yourself as a code contributor.

@ryan-talus
Copy link
Contributor

This change would directly change default behavior, so this would be a potentially unpleasant surprise for people expecting file names to be preserved!

Maybe add this as an option and keep it off by default so it’s not a breaking change?

Because this is part of moving Images to the static directory, it does seem like this is the most elegant place to do it, workarounds would be a bit of a pain.

@ZeldOcarina
Copy link
Author

Hi! Sorry I have been caught up in loads of unexpected work and could not finalize this.
@ryan-talus your idea of adding an option to this is great!!!
If you have time, could you complete the pull request for this? Otherwise I'll get to it as soon as I can, it doesn't seem too difficult to implement.

@moonmeister moonmeister changed the title Remove whitespaces in filenames feat(gatsby-source-airtable): Remove whitespaces in filenames Mar 4, 2023
@moonmeister moonmeister added the enhancement New feature or request label Mar 4, 2023
@moonmeister moonmeister marked this pull request as draft June 30, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants