Skip to content

Various fixes from a copilot review#74

Open
jxy-s wants to merge 5 commits into
mainfrom
copilot-fixes
Open

Various fixes from a copilot review#74
jxy-s wants to merge 5 commits into
mainfrom
copilot-fixes

Conversation

@jxy-s

@jxy-s jxy-s commented Mar 18, 2026

Copy link
Copy Markdown
Collaborator

Copilot found some problems and suggested some changes. They seem reasonable. Tested locally.


Open with Devin

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread VMPlex/UserSettings.cs
// changes. Also, we don't have access to the modern WFP here
// so show an old school message box.
//
if ((DateTime.Now - LastReloadErrorTime).Seconds > 3)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 TimeSpan.Seconds used instead of TotalSeconds for error throttle check

TimeSpan.Seconds returns only the seconds component (0–59) of the time span, not the total elapsed seconds. If more than 60 seconds have passed since the last error (e.g., 62 seconds), .Seconds returns 2, causing 2 > 3 to be false and suppressing the error dialog even though it should be shown. The correct property is .TotalSeconds, which returns the full elapsed time in seconds (e.g., 62.0).

Suggested change
if ((DateTime.Now - LastReloadErrorTime).Seconds > 3)
if ((DateTime.Now - LastReloadErrorTime).TotalSeconds > 3)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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