-
-
Notifications
You must be signed in to change notification settings - Fork 78
docs: improve examples, add new & updated working-with-data doc, reference aw-client examples better
#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
Conversation
- Remove potentially confusing raw_events.py example that performed destructive operations - Create comprehensive working-with-data.rst guide with safety best practices - Add proper redirects for moved pages using sphinxext-rediraffe - Restructure examples section with clearer progression from basic to advanced - Point users to production-ready examples in aw-client repository - Emphasize testing modes and dry-run practices throughout Addresses #1171
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.
Important
Looks good to me! 👍
Reviewed everything up to 9361d59 in 1 minute and 38 seconds. Click for details.
- Reviewed
605lines of code in10files - Skipped
1files when reviewing. - Skipped posting
9draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. Makefile:247
- Draft comment:
Added the 'redirectcheck' target using the rediraffecheckdiff builder. The implementation looks solid – ensure that error outputs are clear if redirects fail. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that error outputs are clear if redirects fail. This falls under the category of asking the author to ensure something, which is against the rules.
2. pyproject.toml:28
- Draft comment:
Dependency 'sphinxext-rediraffe' has been added to support proper redirects. Verify that the specified version (^0.2.7) remains compatible with your Sphinx build. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is about a dependency change, specifically the addition of 'sphinxext-rediraffe'. It asks the PR author to verify compatibility, which is against the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.
3. src/conf.py:44
- Draft comment:
The rediraffe extension is now configured with 'rediraffe_redirects' and 'rediraffe_branch'. Confirm that using 'HEAD~1' is appropriate for your redirect mapping strategy. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment violates the rule about not asking for confirmations or verifications. It's asking the author to confirm their intention with the HEAD1 setting. If there was a specific known issue with using HEAD1, that would be different, but this is just asking for confirmation without providing any concrete reason why it might be problematic. Maybe HEAD1 could actually cause serious issues in some redirect scenarios and this is a valid concern? Maybe this is a common source of problems with rediraffe? Even if HEAD1 could potentially cause issues, the comment doesn't explain what those issues would be or suggest a better alternative. It's just asking for confirmation without providing actionable guidance. The comment should be deleted as it violates the rule about not asking authors to confirm their intentions without providing specific actionable feedback.
4. src/examples.rst:1
- Draft comment:
The examples page now provides a clear roadmap—from canonical events to advanced topics—with a nicely structured toctree. This should improve usability for new users. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, describing changes made to the examples page and its expected impact on usability. It does not provide any actionable feedback or suggestions for improvement.
5. src/examples/querying-data.rst:1
- Draft comment:
This page now clearly indicates that it has been moved and uses an automatic redirect. Consider adding fallback content for users with JavaScript disabled. - Reason this comment was not posted:
Comment looked like it was already resolved.
6. src/examples/working-with-data.rst:1
- Draft comment:
The new 'Working with ActivityWatch Data' guide is comprehensive and well-organized, covering canonical events, custom queries, and raw events along with safety best practices. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, providing feedback on the quality of a guide. It does not offer any actionable suggestions or raise any concerns about the code changes.
7. src/index.rst:39
- Draft comment:
The table of contents has been updated to remove deprecated pages (querying-data, writing-watchers, extending). Ensure that external links and bookmarks are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, as it is asking the author to ensure that external links and bookmarks are updated. It does not provide a specific code suggestion or ask for a specific test to be written. It also does not address any potential issues with the code itself.
8. src/redirects.txt:1
- Draft comment:
A redirect mapping for 'examples/extending.rst' is provided. Confirm that this redirect aligns with the new documentation structure, or consider redirecting to a more specific page if necessary. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm the alignment of a redirect with the new documentation structure. This falls under asking the author to confirm their intention, which is against the rules. The comment does not provide a specific suggestion or point out a clear issue, so it should be removed.
9. Makefile:249
- Draft comment:
Typographical issue: The string "rediraffecheckdiff" (and correspondingly the output directory "$(BUILDDIR)/rediraffe") appears to be a misspelling. It likely should be "redirectcheckdiff" (and "$(BUILDDIR)/redirect") or another correct term. Please verify and correct the spelling. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The term "rediraffe" appears to be an intentional name, likely a Sphinx extension or tool. The consistency between the build target name and directory suggests this is not a typo. Without more context about Sphinx extensions, I should assume the author knows what they're doing. The comment is questioning something that's likely intentional. I could be wrong - maybe it really is a typo and the tool is actually called something else. I don't have complete knowledge of all Sphinx extensions. Even with that uncertainty, the consistency in naming and the fact this is being added as a formal build target strongly suggests this is intentional. We should trust the author's knowledge of their tools. The comment should be deleted as it's questioning something that appears to be intentionally named, and we don't have strong evidence that it's actually incorrect.
Workflow ID: wflow_lPXcpzDTuUMPdTZw
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
…ient.py - Add detailed magic variables example with __CATEGORIES__ usage - Include minimal and hierarchy query examples from original docs - Reference the existing query_client.py practical example - Improve query language documentation while maintaining safety focus
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.
Important
Looks good to me! 👍
Reviewed b2f2b1d in 58 seconds. Click for details.
- Reviewed
56lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/examples/working-with-data.rst:123
- Draft comment:
Good addition of the work activities filtering example. Consider adding a brief inline comment explaining what the helper function 'flood' does, as it may not be immediately clear to all users. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/examples/working-with-data.rst:247
- Draft comment:
Please ensure the file ends with a newline to comply with style guidelines. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_ZzpQE43kOoRJC6uU
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
The querying-data.rst page contained a manual JavaScript redirect, but now that we have sphinxext-rediraffe properly configured in redirects.txt, the extension handles the redirect automatically. This is cleaner and more consistent with standard Sphinx practices.
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.
Important
Looks good to me! 👍
Reviewed df6d178 in 28 seconds. Click for details.
- Reviewed
32lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/examples/querying-data.rst:1
- Draft comment:
Good removal of outdated redirect page. Ensure that the sphinxext-rediraffe configuration fully covers redirection from this file to 'working-with-data', so all old bookmarks are correctly handled. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_TO3gbCPXBkkxpSW5
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Important
Looks good to me! 👍
Reviewed db33da4 in 23 seconds. Click for details.
- Reviewed
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/build.yml:23
- Draft comment:
Graphviz package added; please confirm it's required for docs generation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_gxOJtH96HbS7F4WF
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
working-with-data doc, reference aw-client examples better
Fixes #1171
This PR addresses concerns about potentially confusing example code by significantly improving the documentation structure and safety practices.
Changes
raw_events.pyexample that performed destructive operations without clear warningsworking-with-data.rstguide with three levels:Key Improvements
The new documentation provides a much better user experience while maintaining all functionality.
Important
Improved documentation structure and safety by adding a new guide, restructuring examples, and ensuring proper redirects.
raw_events.pyexample due to unsafe operations.working-with-data.rstguide with sections on Canonical Events, Custom Queries, and Raw Events.examples.rstfor clearer progression from basic to advanced topics.working-with-data.rst.aw-clientrepository.sphinxext-rediraffetopyproject.tomlandconf.pyfor handling redirects.Makefilewithredirectchecktarget to ensure proper redirects.redirects.txtfor managing page redirects.This description was created by
for df6d178. You can customize this summary. It will automatically update as commits are pushed.