Skip to content

@W-18741334: remove hard coded api names for download#200

Merged
unandyala merged 6 commits intofeature/oasfrom
unandyala/remove-hard-coded-apis
Jun 17, 2025
Merged

@W-18741334: remove hard coded api names for download#200
unandyala merged 6 commits intofeature/oasfrom
unandyala/remove-hard-coded-apis

Conversation

@unandyala
Copy link
Contributor

  1. Removes hard-coded API names. Instead it uses the search query to search exchange and download the correct apis
  2. Ensures latest versions are downloaded from multiple version groups

@unandyala unandyala requested a review from a team as a code owner June 11, 2025 19:33
Copy link
Contributor

@joeluong-sfcc joeluong-sfcc left a comment

Choose a reason for hiding this comment

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

LGTM besides these minor comments

* SPDX-License-Identifier: BSD-3-Clause
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
export const PRIMITIVE_DATA_TYPE_MAP = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting that this was never used anywhere, glad we're removing it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't find the usage anywhere

downloadLatestApis(name, PRODUCTION_API_PATH).catch(console.error);
});
// eslint-disable-next-line no-console

Copy link
Contributor

Choose a reason for hiding this comment

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

NIT

Suggested change

// eslint-disable-next-line no-console

downloadLatestApis(
'category:Visibility = "External" category:"SDK Type" = "Commerce" category:"SDK Type" = "Isomorphic"',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why we have SDK type as both Commerce and Isomorphic? If we just query for Isomorphic will it return all the same APIs?

wondering if we should remove Commerce SDK type so its not confusing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just Isomorphic is enough. I remember Kay mentioning to use both. This is kind of ensuring we don't download non-commerce specs that are tagged as Isomorphic (as this is a generic term)

Copy link
Contributor

Choose a reason for hiding this comment

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

Dumb question, but how does this query guarantee that we only pull in the oas specs and not the raml specs?

rootPath: string
): Promise<void> {
const matchedApis = await download.search(`"${name}"`);
const matchedApis = await download.search(searchQuery, undefined, true);
Copy link
Contributor

@joeluong-sfcc joeluong-sfcc Jun 12, 2025

Choose a reason for hiding this comment

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

Can we add a small comment on why we're passing undefined to this search function? Might not be clear to someone who's unfamiliar with these changes

@unandyala unandyala requested a review from vcua-mobify June 16, 2025 13:45
@unandyala unandyala changed the title remove hard coded api names for download @W-18741334: remove hard coded api names for download Jun 16, 2025
Copy link
Contributor

@joeluong-sfcc joeluong-sfcc left a comment

Choose a reason for hiding this comment

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

I don't see the small comment that I requested for

const matchedApis = await download.search(searchQuery, undefined, true);

but otherwise LGTM

Copy link
Contributor

@vcua-mobify vcua-mobify left a comment

Choose a reason for hiding this comment

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

Just had a question on how the query guarantees we only pull down the oas specs / not pull down the raml specs but otherwise it's LGTM

// eslint-disable-next-line no-console

downloadLatestApis(
'category:Visibility = "External" category:"SDK Type" = "Commerce" category:"SDK Type" = "Isomorphic"',
Copy link
Contributor

Choose a reason for hiding this comment

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

Dumb question, but how does this query guarantee that we only pull in the oas specs and not the raml specs?

@unandyala unandyala merged commit a0824ae into feature/oas Jun 17, 2025
4 of 6 checks passed
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