-
Notifications
You must be signed in to change notification settings - Fork 370
Fixes docs contributing guide inconsistencies #7040
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
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.
Pull Request Overview
This PR improves the consistency and clarity of the documentation by standardizing file path formats, fixing formatting issues, and enhancing code examples.
- Standardizes file paths from backslashes to forward slashes for cross-platform compatibility
- Improves list formatting and indentation for better readability
- Fixes typos and enhances error messages in code examples
- Updates validation logic examples to be more concise and readable
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/docs/contribute/new-command/writing-the-docs.mdx | Standardizes file paths to forward slashes, improves CSV output example with explicit boolean values instead of empty strings, enhances list formatting with proper indentation, and adds clarification about alphabetical sorting |
| docs/docs/contribute/new-command/unit-tests.mdx | Adds missing file path in code block title, corrects typo from "occured" to "occurred", improves URL validation in test by using exact match instead of endsWith, and adds missing webUrl parameters |
| docs/docs/contribute/new-command/build-command-logic.mdx | Enhances error messages with quoted URL values and periods, improves option validation logic to be more concise, adds note about alphabetical command sorting, and enhances verbose logging messages |
| docs/docs/contribute/expect-during-pr.mdx | Adds "Successful docs build" to the list of automated checks |
| docs/docs/contribute/contributing-guide.mdx | Contains an incomplete sentence that needs completion |
|
Could we also mention in the contributing guide that when creating a new command, it is necessary to check if the |
Good idea 👍 |
7cd9111 to
af63336
Compare
MartinM85
left a comment
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.
Looks good to me, only one note regarding the sorting that we can discuss.
| ### Include command name | ||
|
|
||
| When you create the minimum file, you'll get an error about a nonexistent type within `commands`. This is correct because we haven't defined the name of the command yet. Let's add this to the `commands` export located in `src/m365/spo/commands.ts`: | ||
| When you create the minimum file, you'll get an error about a nonexistent type within `commands`. This is correct because we haven't defined the name of the command yet. Let's add this to the `commands` export located in `src/m365/spo/commands.ts`. Note that all commands in this file are sorted strictly **alphabetically**. |
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'm not sure about the sorting.
https://github.com/pnp/cli-microsoft365/blob/main/src/m365/spo/commands.ts are sorted alphabetically, but https://github.com/pnp/cli-microsoft365/blob/main/src/m365/entra/commands.ts are sorted by command and subcommand names.
Sorting of Entra commands is more readable, but I'm fine with sorting alphabetically.
af63336 to
d25de82
Compare
Fixes some docs inconsistencies for the
Contributingguide.