Skip to content
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

Make executable (but not really) #100

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make executable (but not really) #100

wants to merge 1 commit into from

Conversation

Bilbottom
Copy link
Owner

@Bilbottom Bilbottom commented Jul 26, 2024

Summary by Sourcery

This pull request introduces command-line argument parsing for debug mode, enhances the SQL view for task rollups, renames classes in Outlook integration modules, and adds detailed logging across the application. Additionally, it updates the README with instructions for building the executable, though this feature is not yet operational.

  • New Features:
    • Added command-line argument parsing to enable debug mode in the main application entry point.
  • Enhancements:
    • Updated SQL view 'tasks_from_yesterday_rollup' to include a summary row with total minutes in hours and minutes format.
    • Renamed 'OutlookInput' class to 'Outlook' in both macOS and Windows Outlook integration modules.
    • Added logging statements to provide more detailed debug information in various parts of the application, including database usage, form creation, and calendar event retrieval.
  • Documentation:
    • Added a new section in the README for building the executable using PyInstaller, although it is noted that this feature is not yet functional.

Copy link
Contributor

sourcery-ai bot commented Jul 26, 2024

Reviewer's Guide by Sourcery

This pull request introduces several changes across multiple files. The most significant changes include the addition of a summary row in the SQL view, renaming the 'OutlookInput' class to 'Outlook' across the codebase, and adding debug logging to various parts of the application. Additionally, a new section for building the executable using PyInstaller has been added to the README, and argparse has been introduced to handle command-line arguments in the main entry point.

File-Level Changes

Files Changes
daily_tracker/integrations/calendars/outlook_mac.py
daily_tracker/integrations/calendars/outlook_windows.py
daily_tracker/integrations/calendars/__init__.py
Renamed 'OutlookInput' class to 'Outlook' and updated relevant imports and references.
daily_tracker/_actions.py
daily_tracker/core/form.py
daily_tracker/main.py
Added debug logging to various parts of the application for better traceability.

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Bilbottom - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider removing the PyInstaller section from the README until it's fully functional to avoid confusion.
  • The SQL view modification in reporting.sql is a nice addition, but consider if there's a more efficient way to achieve this result in a single query.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟡 Documentation: 1 issue found

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

daily_tracker/core/scripts/reporting.sql Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
daily_tracker/__main__.py Show resolved Hide resolved
daily_tracker/integrations/calendars/outlook_mac.py Outdated Show resolved Hide resolved
daily_tracker/integrations/calendars/outlook_mac.py Outdated Show resolved Hide resolved
daily_tracker/__main__.py Show resolved Hide resolved
daily_tracker/__main__.py Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant