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: set host from opt [] #139

Merged
merged 3 commits into from
Feb 13, 2023
Merged

Conversation

crissto
Copy link

@crissto crissto commented Jan 25, 2023

What does it do?

Adds the option to change the host from the opts instead of changing the process.env.BASE_URL being the only way.

Why?

We give the possibility to do this in other clients as https://github.com/contentful/contentful-management.js and it will enable us to use the package in a cleaner way

Risks

Defaults are left untouched, this only changes an optional override so nothing should break

@crissto crissto requested a review from marcolink January 25, 2023 08:59
@crissto crissto requested a review from a team as a code owner January 25, 2023 08:59
@andipaetzold
Copy link
Contributor

Maybe remove the docs for now and we try to find a different solution for them. PRs shouldn't be so big because of docs.

@crissto crissto force-pushed the feat/add-host-opt-to-http-client branch from 880812e to a4a26d5 Compare January 25, 2023 09:05
@@ -145,7 +146,7 @@ export const getManagementToken = (privateKey: string, opts: GetManagementTokenO
}
return createGetManagementToken(
createLogger({ filename: __filename }),
createHttpClient(),
createHttpClient({ prefixUrl: opts.host }),
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to provide a default value for it, or only set it if host is defined.

@crissto crissto requested a review from marcolink February 13, 2023 08:46
Copy link
Member

@marcolink marcolink left a comment

Choose a reason for hiding this comment

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

LGTM

@crissto crissto merged commit 4353c2b into master Feb 13, 2023
@crissto crissto deleted the feat/add-host-opt-to-http-client branch February 13, 2023 13:37
contentful-automation bot added a commit that referenced this pull request Feb 13, 2023
# [2.1.0](v2.0.4...v2.1.0) (2023-02-13)

### Features

* set host from opt [] ([#139](#139)) ([4353c2b](4353c2b))
@contentful-automation
Copy link
Contributor

🎉 This PR is included in version 2.1.0 🎉

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants