Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented May 18, 2025

User description

codeflash all might take a really long time, add a helpful message to let users know how long it might take so they can be patient and plan accordingly.

image

PR Type

Enhancement


Description

  • Add time estimate log for optimize-all

  • Show estimated optimization duration to users


Changes walkthrough 📝

Relevant files
Enhancement
functions_to_optimize.py
Add optimize-all time estimate log                                             

codeflash/discovery/functions_to_optimize.py

  • Insert log message when optimize_all is true
  • Compute duration as functions_count * 3 minutes
  • Notify users about subsequent pull requests
  • +5/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Undefined variable

    Verify that optimize_all is defined and passed into get_functions_to_optimize to avoid runtime errors.

    if optimize_all:
    Magic number

    The multiplier 3 minutes per function is hardcoded; consider extracting it to a constant or configurable setting and documenting the rationale.

    logger.info(
        f"It might take about {functions_count*3} minutes to fully optimize this project. Codeflash "
        f"will keep opening pull requests as it finds optimizations."

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Handle minute pluralization

    Add conditional pluralization for "minute" when the total estimate equals one minute
    to ensure grammatically correct logging.

    codeflash/discovery/functions_to_optimize.py [206-210]

     if optimize_all:
    +    total_minutes = functions_count * 3
         logger.info(
    -        f"It might take about {functions_count*3} minutes to fully optimize this project. Codeflash "
    -        f"will keep opening pull requests as it finds optimizations."
    +        f"It might take about {total_minutes} minute{'s' if total_minutes != 1 else ''} to fully optimize this project. Codeflash "
    +        "will keep opening pull requests as it finds optimizations."
         )
    Suggestion importance[1-10]: 4

    __

    Why: The suggestion improves logging grammar by handling singular minute but is a minor benefit with low overall impact.

    Low

    @misrasaurabh1 misrasaurabh1 requested a review from a team May 18, 2025 04:38
    @aseembits93
    Copy link
    Contributor

    @misrasaurabh1 minutes to hours:minutes or day/s:hours:minutes when the number is large

    def fmt_days_hm(total_minutes: int) -> str:
        days, rem_minutes = divmod(total_minutes, 1440)      # 1440 = 24*60
        hours, minutes   = divmod(rem_minutes, 60)
        return f"{days}:{hours:02d}:{minutes:02d}"
    

    Copy link
    Contributor

    @aseembits93 aseembits93 left a comment

    Choose a reason for hiding this comment

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

    Approving, although I don't see a unit for 'days' in humanize_runtime. Will modify it in a future PR

    @misrasaurabh1 misrasaurabh1 merged commit 113f59d into main May 18, 2025
    15 checks passed
    @aseembits93 aseembits93 deleted the add-optimize-all-time-estimate branch May 18, 2025 05:26
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants