-
-
Notifications
You must be signed in to change notification settings - Fork 40
Refresh the documentation website #152
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?
Refresh the documentation website #152
Conversation
WalkthroughDocumentation files are updated with YAML front matter to hide navigation, heading levels are standardized across the site, and a new MkDocs configuration file is added to establish the documentation structure and theming for the MVIKotlin project. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/store.md (1)
62-62: Use "cannot" instead of "can not" for consistency.LanguageTool suggests using "cannot" (one word) instead of "can not" (two words) for more formal and common style.
Apply this diff:
-> ⚠️ Please note that `Bootstrappers` are stateful and so cannot be `object`s (singletons). -> ⚠️ Please note that `Executors` are stateful and so cannot be `object`s (singletons). -> ⚠️ Please note that `Executors` are stateful and so cannot be `object`s (singletons).Also applies to: 68-68, 74-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/media/multipleStores.pngis excluded by!**/*.png
📒 Files selected for processing (8)
docs/binding_and_lifecycle.md(2 hunks)docs/index.md(2 hunks)docs/logging.md(1 hunks)docs/state_preservation.md(2 hunks)docs/store.md(15 hunks)docs/time_travel.md(11 hunks)docs/view.md(2 hunks)mkdocs.yml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/time_travel.md
[style] ~191-~191: For conciseness, consider replacing this expression with an adverb.
Context: ...trol the feature and displays the data. At the moment, there are three variants of the client...
(AT_THE_MOMENT)
[style] ~223-~223: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...n/Apple applications. >
(CAN_NOT_PREMIUM)
docs/index.md
[style] ~43-~43: To make your writing clearer, consider a more direct alternative.
Context: ...optional ## How the data flows Please take a look at the following diagram: <img sr...
(TAKE_A_LOOK)
[style] ~51-~51: Consider an alternative for the often overused word ‘important’.
Context: ...r for any other actions that are not so important to be part of the State. The data fl...
(NOT_IMPORTANT)
docs/store.md
[style] ~62-~62: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...hat Bootstrappers are stateful and so can not be objects (singletons). ## Executor...
(CAN_NOT_PREMIUM)
[style] ~68-~68: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...te that Executors are stateful and so can not be objects (singletons). ## Executor...
(CAN_NOT_PREMIUM)
[style] ~74-~74: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...te that Executors are stateful and so can not be objects (singletons). ## Reducer ...
(CAN_NOT_PREMIUM)
🪛 markdownlint-cli2 (0.18.1)
docs/time_travel.md
231-231: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/index.md
12-12: Images should have alternate text (alt text)
(MD045, no-alt-text)
45-45: Images should have alternate text (alt text)
(MD045, no-alt-text)
🔇 Additional comments (9)
mkdocs.yml (1)
2-2: Update placeholder domain in site_url configuration.The
site_urlis set to "https://mydomain.org/mvikotlin", which appears to be a placeholder. Ensure this is updated to the actual domain where the documentation will be hosted.docs/logging.md (1)
1-12: LGTM!The YAML front matter and heading-level adjustments align with the documentation restructuring across the site. Navigation will be hidden, and the heading hierarchy is consistent.
docs/view.md (1)
1-98: LGTM!Consistent with the site-wide documentation refresh. YAML front matter hides navigation, and heading levels align with the new structure.
docs/state_preservation.md (1)
1-120: LGTM!Front matter and heading restructuring align with the site-wide refresh. Content organization with "Retaining objects" as a main subsection improves clarity and hierarchy consistency.
docs/index.md (1)
1-4: LGTM!YAML front matter and heading restructuring are consistent with the site-wide documentation refresh.
docs/store.md (2)
1-6: LGTM!YAML front matter and heading restructuring align with the site-wide documentation refresh.
700-727: New section provides clear multi-store pattern example.The "Using Multiple Stores on a Single Screen" section effectively demonstrates a practical use case with well-structured code. The example clearly shows how to compose multiple independent stores and manage data flows between them.
docs/binding_and_lifecycle.md (1)
1-106: LGTM!YAML front matter and heading restructuring are consistent with the site-wide refresh. The addition of explicit
stateToModelandeventToIntentmapper examples clarifies the binding process and improves documentation clarity. The lifecycle-aware binding example effectively demonstrates best practices.docs/time_travel.md (1)
1-6: LGTM!YAML front matter and heading restructuring align with the site-wide documentation refresh. Content organization and platform-specific sections are clearly presented.
|
|
||
| MVI stands for Model-View-Intent. It is an architectural pattern that utilizes unidirectional data flow. The data circulates between `Model` and `View` only in one direction - from `Model` to `View` and from `View` to `Model`. | ||
|
|
||
| <img src="media/mvi.jpg" width="256"> |
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.
Add alt text to image.
The image is missing descriptive alt text for accessibility.
Apply this diff to add alt text:
-<img src="media/mvi.jpg" width="256">
+<img src="media/mvi.jpg" width="256" alt="MVI pattern diagram">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img src="media/mvi.jpg" width="256"> | |
| <img src="media/mvi.jpg" width="256" alt="MVI pattern diagram"> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
12-12: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In docs/index.md around line 12, the <img> tag lacks alt text which is required
for accessibility; update the tag to include a meaningful alt attribute (e.g.,
alt="MVI architecture diagram" or another concise descriptive phrase appropriate
for the image) so screen readers and assistive tech can convey the image's
content.
|
|
||
| Please take a look at the following diagram: | ||
|
|
||
| <img src="media/mvikotlin.jpg" width="384"> |
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.
Add alt text to image.
The image is missing descriptive alt text for accessibility.
Apply this diff to add alt text:
-<img src="media/mvikotlin.jpg" width="384">
+<img src="media/mvikotlin.jpg" width="384" alt="MVIKotlin architecture diagram">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img src="media/mvikotlin.jpg" width="384"> | |
| <img src="media/mvikotlin.jpg" width="384" alt="MVIKotlin architecture diagram"> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
45-45: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In docs/index.md around line 45 the <img> tag lacks alt text; update the tag to
include a concise, descriptive alt attribute (e.g., alt="MVIKotlin logo" or
another appropriate description) so the image is accessible to screen readers
and follows accessibility best practices.
| ``` | ||
| ./gradlew :mvikotlin-timetravel-client:app-desktop:run | ||
| ``` |
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.
Add language identifier to fenced code block.
Code blocks should specify a language for proper syntax highlighting. This block should be marked as bash or shell.
Apply this diff:
-```
+```bash
./gradlew :mvikotlin-timetravel-client:app-desktop:run
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
231-231: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In docs/time_travel.md around lines 231 to 233, the fenced code block lacks a
language identifier; update the opening fence from ``` to ```bash so the block
becomes a bash/shell code block for proper syntax highlighting, leaving the
inner command and closing fence unchanged.
Summary by CodeRabbit