-
Notifications
You must be signed in to change notification settings - Fork 57
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: retrieve integration meta data from new endpoint #5647
feat: retrieve integration meta data from new endpoint #5647
Conversation
This pull request adds or modifies JavaScript ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good :)
packages/config/src/api/site_info.ts
Outdated
if (useV2Endpoint) { | ||
const promises = [ | ||
getAccounts(api), | ||
getAddons(api, siteId), | ||
getIntegrations({ siteId, testOpts, offline, accountId: siteInfo.account_id, featureFlags }), | ||
] | ||
|
||
siteInfo.build_settings.env = envelope | ||
} | ||
const [accounts, addons, integrations] = await Promise.all(promises) | ||
|
||
if (siteInfo.use_envelope) { | ||
const envelope = await getEnvelope({ api, accountId: siteInfo.account_slug, siteId, context }) | ||
|
||
siteInfo.build_settings.env = envelope | ||
} | ||
|
||
return { siteInfo, accounts, addons, integrations } | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's about mode 'buildbot' shouldn't be this covered in this part as well?
Why is the mode buildbot only covered in the non feature flaged way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am honest, I don't know why we did this and I opted to keep this as it was previously written so that I could be sure I didn't regress the previous functionality as I am only interested in the adding the v2 behaviour
🎉 Thanks for submitting a pull request! 🎉
Summary
Integrations are changing and this also means the endpoint we use to fetch information about integrations installed on a site is also changing. This points to the new endpoint behind a feature flag.
For us to review and ship your PR efficiently, please perform the following steps:
we can discuss the changes and get feedback from everyone that should be involved. If you`re fixing a typo or
something that`s on fire 🔥 (e.g. incident related), you can skip this step.
your code follows our style guide and passes our tests.
A picture of a cute animal (not mandatory, but encouraged)