-
-
Notifications
You must be signed in to change notification settings - Fork 276
docs(testing): grammar and clarity improvements #2685
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: master
Are you sure you want to change the base?
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughEditorial revisions to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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.
Pull request overview
This PR improves the documentation quality of the testing guide by correcting grammatical errors, fixing typos, and enhancing clarity throughout the document. The changes make the testing documentation more professional and easier to understand.
Key changes:
- Corrected grammatical errors and improved sentence structure
- Fixed spelling mistakes and typos (including a broken URL)
- Enhanced clarity by adding links to related documentation and improving technical descriptions
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/getting-started/testing.mdx (1)
190-190: Consider reducing wordiness in semantic explanation.The phrase "take into account that" can be more concise. Consider restructuring to improve readability.
-Semantics for above check is: whether `user:1` can push to `repository:3`, in addition to stored tuples, take into account that user:1 is owner of repository:3 (`repository:3#owner@user:1`). Expected result for that check it **true** - `push : true` +Semantics for above check is: whether `user:1` can push to `repository:3`, in addition to stored tuples, assuming user:1 is owner of repository:3 (`repository:3#owner@user:1`). Expected result for that check is **true** - `push : true`Note: This diff also corrects "it true" → "is true" (another typo in the same segment).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/getting-started/testing.mdx(8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/testing.mdx
[grammar] ~154-~154: Ensure spelling is correct
Context: ...a) according to a schema. Next, you'll examinine how to create scenarios. ## Creating T...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[style] ~190-~190: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...itory:3, in addition to stored tuples, take into account that user:1 is owner of repository:3 (...
(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
🔇 Additional comments (1)
docs/getting-started/testing.mdx (1)
7-7: Grammar and clarity improvements approved.The remaining editorial changes throughout the file (grammar corrections, article additions, link refinements, terminology consistency, and explanatory clarifications) are well-executed and improve the documentation's readability and professionalism.
Also applies to: 43-46, 152-152, 162-162, 214-215, 231-231, 239-239, 243-243, 273-273, 304-304
additional typo fixes Co-authored-by: Copilot <[email protected]>
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/getting-started/testing.mdx(8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/getting-started/testing.mdx
[style] ~190-~190: ‘take into account’ might be wordy. Consider a shorter alternative.
Context: ...itory:3, in addition to stored tuples, take into account that user:1 is owner of repository:3 (...
(EN_WORDINESS_PREMIUM_TAKE_INTO_ACCOUNT)
🔇 Additional comments (11)
docs/getting-started/testing.mdx (11)
7-7: Strong introductory edit. Adding "critical" emphasizes the importance of testing without compromising clarity.
43-46: Correctly addresses past review feedback. The article "of" is now properly included ("consists of 3 parts"), and the reformatted bullet points improve readability.
152-152: Clear cross-referencing. The parenthetical clarifications and explicit links help readers navigate related concepts effectively.
154-154: Spelling correction confirmed. The word "examine" is now spelled correctly (previously was "examinine"), addressing previous review feedback.
162-162: Enhanced clarity. Adding "configuration" clarifies what aspect of scenarios is being examined.
214-215: Improved practical guidance. Concrete examples (3-5 for shallow, 20-50 for deep) make the depth configuration recommendations more actionable.
231-231: Clarified field purpose. The added phrase "for which you want to perform data filtering" improves understanding of thesubjectparameter's role.
239-239: Expanded explanation improves comprehension. The detailed breakdown of howentity_filtersdiffers fromchecksin assertions makes the distinction clearer for readers.
243-243: Practical examples clarify intent. The "such as which users can perform action Y" examples help readers understand what subject filtering queries are used for.
273-273: Minor grammar note. The phrase "For that open up" could read slightly better as "To do that, open up" or "For that, open a new file" (adding a comma after "that"), but the meaning is clear and the instruction sequence is effective.
304-304: Polished CTA. The refined wording maintains a friendly tone while clearly directing users to the consultation resource.
nathan-contino
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.
LGTM, some minor suggestions. Reach out to me if you want to chat about any of them!
| --- | ||
|
|
||
| Testing is critical process when building and maintaining an authorization system. This page explains how to ensure the new authorization model and related authorization data works as expected in Permify. | ||
| Testing is a critical process when building and maintaining an authorization system. This page explains how to ensure the new authorization model and related authorization data works as expected in Permify. |
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.
| Testing is a critical process when building and maintaining an authorization system. This page explains how to ensure the new authorization model and related authorization data works as expected in Permify. | |
| Testing is a critical process when building and maintaining an authorization system. This page explains how to ensure your authorization model and data works as expected in Permify. |
[suggestion]: reduce duplication of 'authorization' in this paragraph. 3 times is probably too many in such a small paragraph!
| - `schema` which is the authorization model you want to test | ||
| - `relationships` which is the sample data to test your model | ||
| - `scenarios` which is the way to actually test access check queries within created scenarios |
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.
| - `schema` which is the authorization model you want to test | |
| - `relationships` which is the sample data to test your model | |
| - `scenarios` which is the way to actually test access check queries within created scenarios | |
| - `schema`: the authorization model you want to test | |
| - `relationships`: the sample data to test your model | |
| - `scenarios`: the way to actually test access check queries within created scenarios |
[suggestion]: why use more word when few word do trick?
| - `schema` which is the authorization model you want to test, | ||
| - `relationships` sample data to test your model, | ||
| - `scenarios` to test access check queries within created scenarios. | ||
| Below, you can examine an example schema validation yaml file. It consists of 3 parts; |
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.
| Below, you can examine an example schema validation yaml file. It consists of 3 parts; | |
| The example schema validation yaml below consists of the following parts: |
- reduce wordiness
- remove specific reference to "3 parts" since it's very possible that future updates will change that number, but we might not remember to update this sentence
- use a colon to introduce a bulleted list, instead of a semicolon (nitpick)
| ``` | ||
| Assuming that you're well-familiar with the `schema` and `relationships` sections of the above YAML file. If not, please see the previous sections to learn how to create an authorization model (schema) and generate data (relationships) according to it. | ||
| You should be familiar with the `schema` and `relationships` sections of the above YAML file. If not, please see the previous sections to learn how to [create an authorization model (the schema)](/getting-started/modeling) and [generate data (relationships)](/getting-started/sync-data) according to a schema. |
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.
| You should be familiar with the `schema` and `relationships` sections of the above YAML file. If not, please see the previous sections to learn how to [create an authorization model (the schema)](/getting-started/modeling) and [generate data (relationships)](/getting-started/sync-data) according to a schema. | |
| For more information about the `schema` and `relationships` sections of the above YAML file, see [create an authorization model (the schema)](/getting-started/modeling) and [generate data (relationships)](/getting-started/sync-data). |
[suggestion]: reduce wordiness; phrases like 'you should be familiar with' are mostly filler. let's just get straight to the point instead!
| You should be familiar with the `schema` and `relationships` sections of the above YAML file. If not, please see the previous sections to learn how to [create an authorization model (the schema)](/getting-started/modeling) and [generate data (relationships)](/getting-started/sync-data) according to a schema. | ||
|
|
||
| We'll continue by examining how to create scenarios. | ||
| Next, you'll examine how to create scenarios. |
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.
| Next, you'll examine how to create scenarios. |
The subsequent header already does the legwork of introducing this idea, let's reduce redundancy.
| You can also test your new authorization model in your local (Permify clone) without using [permify-validate-action] at all. | ||
|
|
||
| For that open up a new file and add a schema yaml file inside. Then build your project with, run `make build` command and run `./permify validate {path of your schema validation file}`. | ||
| For that open up a new file and add a schema yaml file inside. Then build your project, run `make build` command and run `./permify validate {path of your schema validation file}`. |
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.
| For that open up a new file and add a schema yaml file inside. Then build your project, run `make build` command and run `./permify validate {path of your schema validation file}`. | |
| To test locally: | |
| 1. Create a schema yaml file. | |
| 1. Build your project )(`make build`). | |
| 1. To validate the schema, run `./permify validate <path-to-schema-validation-file>`. |
3 steps is typically where I draw the line and create an ordered list. Also, I think 'build your project' and 'run make build command' are the same step, so even if we don't break this into discrete steps, we ought to ditch that comma!
| ## Need any help ? | ||
|
|
||
| Our team is happy to help you get started with Permify. If you'd like to learn more about using Permify in your app or have any questions about it, [schedule a consultation call with one of our account executives](hhttps://calendly.com/d/cj79-kyf-b4z). | ||
| Our team is happy to help you get started with Permify. If you'd like to learn more about using Permify in your app or have any questions about it, [schedule a consultation call with one of our account executives](https://calendly.com/d/cj79-kyf-b4z). |
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 have to admit, I would have missed this one! Nice eye.
Updated the testing page. Added some links but mostly corrected some typos.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.