-
Notifications
You must be signed in to change notification settings - Fork 4
README update for packages #771
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
base: main
Are you sure you want to change the base?
Conversation
@@ -1,11 +1,193 @@ | |||
# `@microsoft/msgraph-sdk` | |||
# Microsoft Graph SDK for Typescript |
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.
This is a copy of the root README
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.
I'd advise against keeping a duplicate copy - it increases the likelihood of them getting out of sync. If you do decide to keep it, all the notes I made on the root README apply ;)
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.
primary reason is because we will not be able to get the documentation in npm if we do not have a README, @baywet what are your thoughts, should we keep this one?
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.
@jasonjoh the problem we're trying to solve is that the package page on npm is empty. https://www.npmjs.com/package/@microsoft/msgraph-sdk-users
We could alternative maintain a template, at a central location and have a script that copies the file before npm pack.
There are some small replacements that are needed but that should be manageable. Thoughts?
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.
@baywet I like the idea of having a template, we can copy the README from the root directory to the package msgraph-sdk
but the rest of the packages can have slightly different README
files, similar to the admin package
@@ -1,11 +1,80 @@ | |||
# `@microsoft/msgraph-sdk-admin` |
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.
Should be duplicated in all the subfolders
@@ -1,11 +1,193 @@ | |||
# `@microsoft/msgraph-sdk` | |||
# Microsoft Graph SDK for Typescript |
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.
I'd advise against keeping a duplicate copy - it increases the likelihood of them getting out of sync. If you do decide to keep it, all the notes I made on the root README apply ;)
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.
I just realized that I never posted my comments...
packages/msgraph-sdk-admin/README.md
Outdated
const requestAdapter = new GraphRequestAdapter(authProvider); | ||
|
||
// Create a GraphServiceClient | ||
const graphServiceClient = createGraphServiceClient(requestAdapter); |
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.
I'd have expected this to demonstrate using the admin client instead in case they use this package directly.
Same thing for the import
@@ -1,11 +1,193 @@ | |||
# `@microsoft/msgraph-sdk` | |||
# Microsoft Graph SDK for Typescript |
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.
@jasonjoh the problem we're trying to solve is that the package page on npm is empty. https://www.npmjs.com/package/@microsoft/msgraph-sdk-users
We could alternative maintain a template, at a central location and have a script that copies the file before npm pack.
There are some small replacements that are needed but that should be manageable. Thoughts?
dd0e70e
to
1d1f0ef
Compare
Closes #485
Draft README update for nested packages