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

docs: Add examples for all languages into the documentation #270

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

hwclass
Copy link

@hwclass hwclass commented Apr 16, 2024

Screenshot 2024-04-17 at 08 49 43

@vmwclabot
Copy link

@hwclass, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@vmwclabot
Copy link

@hwclass, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

Copy link
Member

@ereslibre ereslibre left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your contribution @hwclass! It LGTM 👏, just one comment regarding the lock file and I think we can merge it, once the CLA process has been completed. Once merged I'll take care of publishing the docs :)

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 we can avoid adding the package lock; can you please add it to the .gitignore file?

Copy link
Author

@hwclass hwclass Apr 22, 2024

Choose a reason for hiding this comment

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

Hi @ereslibre

Thanks for your message. I think we need to keep track of the metadata of the packages installed as how it has been mentioned in npm's docs here: https://docs.npmjs.com/cli/v10/configuring-npm/package-lock-json#description. wdyt? If there is/are some concerns regarding that, I can act based upon the suggestion, but that's the best practice in general.

Once you give the thumbsup, we're good to go, I think 👍

Copy link
Member

Choose a reason for hiding this comment

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

In this case it shouldn't be relevant, because we are using docusaurus to generate the documentation, and that dependency is pinned to a specific version, so the package.lock generates more noise than it solves in this case, IMO.

Before we can move forward we have to wait for the CLA to go through, thanks again for your contribution! 👍

Copy link
Author

Choose a reason for hiding this comment

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

hey @ereslibre, ok then, just pushed the updated commit. Thanks for your comments 🙌

Copy link
Author

Choose a reason for hiding this comment

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

Hey @ereslibre

It is already 3 weeks, is there any progress on the CLA or there is sth. else that I need to do to move forward with the PR?

fyi. @flavio @nebhale

Copy link
Member

Choose a reason for hiding this comment

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

Hello @hwclass. First of all, thank you for your contribution and I'm sorry it's taking this long for the CLA to be sorted out.

Let me find out and I'll come back to you as soon as I know what's the reason for the delay.

@vmwclabot
Copy link

@hwclass, VMware has approved your signed contributor license agreement.

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.

None yet

3 participants