Skip to content

properly handle deleting sha from query parameters#5009

Merged
auto-submit[bot] merged 4 commits intomainfrom
184372-pr
Mar 31, 2026
Merged

properly handle deleting sha from query parameters#5009
auto-submit[bot] merged 4 commits intomainfrom
184372-pr

Conversation

@ievdokdm
Copy link
Copy Markdown
Contributor

@ievdokdm ievdokdm requested a review from eyebrowsoffire March 31, 2026 01:52
@github-actions
Copy link
Copy Markdown
Contributor

🤖 Hi @ievdokdm, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

## 📋 Review Summary

The Pull Request successfully addresses the issue of properly handling the deletion of the SHA from query parameters in the dashboard. The introduction of asynchronous logic and the reset of internal state ensure that the latest SHA is correctly picked when the user removes the parameter from the URL.

🔍 General Feedback

  • The change to make many PresubmitState methods async and return Future<void> is consistent and well-applied across both the main code and the tests.
  • However, there are a few places where await might be over-applied, potentially causing unnecessary UI delays or misleading loading indicators (e.g., when filtering or fetching summaries). Reverting those to unawaited would improve the user experience while maintaining the correctness of the state updates.
  • Overall, the logic is sound and the added tests adequately cover the new behavior.

@ievdokdm ievdokdm added the CICD Run CI/CD label Mar 31, 2026
@ievdokdm ievdokdm added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 31, 2026
@ievdokdm ievdokdm requested a review from eyebrowsoffire March 31, 2026 18:16
@ievdokdm ievdokdm added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 31, 2026
Copy link
Copy Markdown
Contributor

@eyebrowsoffire eyebrowsoffire left a comment

Choose a reason for hiding this comment

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

LGTM!

@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit bot commented Mar 31, 2026

auto label is removed for flutter/cocoon/5009, Failed to merge flutter/cocoon/5009 with Pull request flutter/cocoon/5009 could not be merged: Required status check "ci.yaml validation" is expected..

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 31, 2026
@ievdokdm ievdokdm added the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 31, 2026
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App. label Mar 31, 2026
@auto-submit
Copy link
Copy Markdown
Contributor

auto-submit bot commented Mar 31, 2026

auto label is removed for flutter/cocoon/5009, Failed to merge flutter/cocoon/5009 with Pull request flutter/cocoon/5009 could not be merged: Required status check "ci.yaml validation" is expected..

@ievdokdm ievdokdm added autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD and removed CICD Run CI/CD labels Mar 31, 2026
@auto-submit auto-submit bot merged commit 25fe8ec into main Mar 31, 2026
10 checks passed
@ievdokdm ievdokdm deleted the 184372-pr branch March 31, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App. CICD Run CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Open dashboard using PR number does not automatically select latest sha in GuardStatus dropdown

2 participants