Skip to content

feat(api): improvements to the installation screen - #783

Merged
erunion merged 2 commits into
mainfrom
post-install-code-example
Oct 24, 2023
Merged

feat(api): improvements to the installation screen#783
erunion merged 2 commits into
mainfrom
post-install-code-example

Conversation

@erunion

@erunion erunion commented Oct 24, 2023

Copy link
Copy Markdown
Member
🚥 Resolves RM-8185

🧰 Changes

This work is some QOL improvements to the post-installation screen where we'll now, using a very simple algorithm to determine the best endpoint for a demo1, show a code example to folks so they can immediately get started with their newly generated SDK. Here's what it looks like:

Screen Shot 2023-10-23 at 5 02 24 PM

And in the case of us not being able to find a good endpoint to demo2 we just won't show a code example:

Screen Shot 2023-10-23 at 5 02 43 PM

Footnotes

  1. With the exception of us being able to handle auth examples, this algorithm is the same that powers the "recommended" API endpoint section of ReadMe Realtime configuration.

  2. In this case this API definition was only comprised endpoints with path parameters.

@erunion erunion added enhancement New feature or request area:api Issues related to the `api` CLI, which builds the SDKs labels Oct 24, 2023
}));
}
.action(async (uri: string, options: Options) => {
const language = await getLanguage(options);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just some very small refactors to move this language and identifier selection out so this command action is a little easier to read.

Comment thread packages/httpsnippet-client-api/src/index.ts
return false;
}

const snippet = client.convert(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't love this but I also really don't want to have to make @readme/oas-to-snippet a dependency because that depends on httpsnippet-client-api and if we need to make fixes to this snippet generation work it's be a multistep process:

  1. publishing httpsnippet-client-api
  2. pulling that release into @readme/oas-to-snippet
  3. publishing @readme/oas-to-snippet.
  4. puling that release into api
  5. publishing api.

});

return `sdk.auth(${auth.join(', ')});`;
return `${sdkVariable}.auth(${auth.join(', ')});`;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

found a bug in these .auth() snippets with custom variables while building this out.

@erunion
erunion marked this pull request as ready for review October 24, 2023 00:13
@erunion
erunion requested a review from kanadgupta October 24, 2023 00:13
@erunion erunion added this to the v7 milestone Oct 24, 2023

@kanadgupta kanadgupta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

changes look good, really liking some of the refactors here! one small wrinkle: if the user's identifier has a hyphen, we should probably convert it to camelcase (or underscores, whatever you think makes the most sense!)

- import transit-new from '@api/transit-new';
+ import transitNew from '@api/transit-new';

- transit-new.getNextripAgencies()
+ transitNew.getNextripAgencies()
  .then(({ data }) => console.log(data))
  .catch(err => console.error(err));

@erunion
erunion requested a review from kanadgupta October 24, 2023 19:31
if (isReservedOrBuiltinsLC(sdkVariable)) {
// If this identifier is a reserved JS word then we should prefix it with an underscore so
// this snippet can be valid code.
sdkVariable = `_${sdkVariable}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

great call here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah we do something similar for method and type names in codegen

@erunion
erunion merged commit 3280f1f into main Oct 24, 2023
@erunion
erunion deleted the post-install-code-example branch October 24, 2023 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:api Issues related to the `api` CLI, which builds the SDKs enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants