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

Remove unused CONTENTFUL_HOST environment variable #9344

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

twogood
Copy link

@twogood twogood commented Dec 17, 2024

Purpose

Solve problem for Windows user that posted in Contentful Discord:

https://discord.com/channels/1030176999471321129/1317987388567457835

Approach

Use cross-env so that the scripts works on Windows too.

Note: CONTENTFUL_HOST is read by the host variable in http.ts but then the host variable is not actually used in http.ts.

Testing steps

Run affected scripts at command line.

Breaking Changes

No.

Dependencies and/or References

No.

Deployment

No, this is example code.

Also it was set without using cross-env, causing problems for Windows users.
@twogood twogood requested a review from a team as a code owner December 17, 2024 18:16
Copy link

netlify bot commented Dec 17, 2024

👷 Deploy request for ecommerce-app-base-components pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit fa39ce8

Copy link

@Seth-Carter Seth-Carter left a comment

Choose a reason for hiding this comment

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

Let's just leave it as is and go head and use cross-env.

examples/native-external-references/package.json Outdated Show resolved Hide resolved
examples/native-external-references/package.json Outdated Show resolved Hide resolved
examples/native-external-references/src/tools/http.ts Outdated Show resolved Hide resolved
Seth-Carter and others added 4 commits December 18, 2024 08:43
…ve-external-references-clean-up-for-windows-users
…of github.com:twogood/contentful-apps into native-external-references-clean-up-for-windows-users
@twogood
Copy link
Author

twogood commented Dec 18, 2024

Let's just leave it as is and go head and use cross-env.

PR updated based on feedback. Commit history got weird though.

@useyourillusions
Copy link

Hi!
Cross-env could be a great solution for cases where we truly need this environment variable, but it's entirely unusable within the source code. Wouldn't it make more sense to remove it?

@twogood twogood requested a review from Seth-Carter December 18, 2024 20:42
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