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

SDK quickstart structure (done on NodeJS) #373

Merged
merged 18 commits into from
Jul 15, 2024
Merged

Conversation

filipermit
Copy link
Collaborator

No description provided.

@filipermit filipermit marked this pull request as draft June 21, 2024 19:44
Copy link

netlify bot commented Jun 21, 2024

Deploy Preview for permitio-docs ready!

Name Link
🔨 Latest commit 6a59913
🔍 Latest deploy log https://app.netlify.com/sites/permitio-docs/deploys/668da91e3d695b00085419f9
😎 Deploy Preview https://deploy-preview-373--permitio-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@gemanor gemanor left a comment

Choose a reason for hiding this comment

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

This is a good start, please see comments.

docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
@filipermit filipermit requested a review from gemanor June 25, 2024 09:28
Copy link
Collaborator

@danielbass37 danielbass37 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@gemanor gemanor left a comment

Choose a reason for hiding this comment

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

LGTM left some comments. Please address them (let me know if anything needs to be clarified).

After that, please:

  1. Save a version of the outline in Notion or somewhere we can copy it for next versions
  2. Make sure to remove all the language placeholders
  3. Make the quickstart page as the main page of the Node.js SDK
  4. Remove theNode.js SDK examples page and redirect it to here
  5. Add all the existing pages under Node.js in the bottom of the page using the Docusaurus element that shows other sub pages

We still need to think if we want to restructure the other pages under the SDK or keep them as-is.

docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
@filipermit filipermit marked this pull request as ready for review July 3, 2024 11:38
@filipermit filipermit requested a review from gemanor July 4, 2024 08:22
Copy link
Collaborator

@gemanor gemanor left a comment

Choose a reason for hiding this comment

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

Another good progress, left some comments.
Pay attention that you haven't addressed the 3 and 5 points from the previous review.

docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
docs/sdk/quickstart-by-language/quickstart-nodejs.mdx Outdated Show resolved Hide resolved
@filipermit
Copy link
Collaborator Author

@gemanor - can you please elaborate points 3 & 5 in more detail - I've addressed them to how I understood them, but it looks like you have something more in mind.

For 3 - I replaced the current Node.js quickstart page with the latest quickstart page
For 5 - I changed the structure of the navigation to include the sub-pages under the quickstart - do you mean adding tiles to the bottom of the quickstart page which link to the other sub mdx pages for nodejs?

@filipermit filipermit requested a review from gemanor July 5, 2024 20:57
Copy link
Collaborator

@gemanor gemanor left a comment

Choose a reason for hiding this comment

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

LGTM. Nice job 👍👏

@filipermit filipermit merged commit da0d7f0 into master Jul 15, 2024
6 checks passed
@filipermit filipermit deleted the new-sdk-structure branch July 15, 2024 13:31
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