Skip to content

cleanup: dart 3.10, dot shorthands, refactor isSuccess#4991

Merged
auto-submit[bot] merged 6 commits into
mainfrom
mqgClosedOnSuppressed
Mar 19, 2026
Merged

cleanup: dart 3.10, dot shorthands, refactor isSuccess#4991
auto-submit[bot] merged 6 commits into
mainfrom
mqgClosedOnSuppressed

Conversation

@jtmcdole
Copy link
Copy Markdown
Member

A number of things:

  1. Update to dart 3.10 to allow for dart dot-shorthands
  2. Refactor "isBuildSuccessed" and "isBuildFailed" - there is already an isSuccess, isCompleted, and isFailure.

- update to dart 3.10
- use dot shorthands in some places
- refactor "status" maps to work.
@jtmcdole jtmcdole requested a review from ievdokdm March 19, 2026 17:55
@github-actions
Copy link
Copy Markdown
Contributor

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

@jtmcdole
Copy link
Copy Markdown
Member Author

There is definitely more places we can update / cleanup. I'll probably write a task to do it.

@jtmcdole jtmcdole added the CICD Run CI/CD label Mar 19, 2026
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

This pull request successfully refactors TaskStatus and TaskConclusion states to consolidate checks for success, completion, and failure statuses while bringing in Dart 3.10 features. However, several critical compile-time errors have been identified where dot shorthand (.enumValue) is improperly used in combination with the equality (== and !=) operators.

🔍 General Feedback

  • Dot Shorthand Limitations: The == and != operators in Dart take an Object as a parameter. Because of this, they do not provide an expected context type to the right-hand side operand, rendering dot shorthand invalid and causing compile-time errors.
  • Enum Lexical Scope: As a general rule when working within an enum block, its constants are already available in lexical scope, so you can just use constantName without qualification or dot shorthand (e.g., success instead of .success or TaskConclusion.success).
  • Use Switch Expressions: The refactoring to use switch (this) for boolean status getters (like isSuccess and isFailure) is clean, idiomatic Dart 3 code that leverages pattern matching. Where applicable, switch blocks provide the required context type that makes dot shorthand perfectly valid and should be used instead of chained equality checks.

Comment thread app_dart/lib/src/model/firestore/ci_staging.dart
Comment thread app_dart/lib/src/model/firestore/ci_staging.dart Outdated
Comment thread app_dart/lib/src/model/firestore/ci_staging.dart Outdated
Comment thread app_dart/lib/src/model/firestore/ci_staging.dart
Comment thread app_dart/lib/src/request_handlers/presubmit_luci_subscription.dart
Comment thread packages/cocoon_common/lib/task_status.dart Outdated
@jtmcdole jtmcdole added CICD Run CI/CD and removed CICD Run CI/CD labels Mar 19, 2026
Copy link
Copy Markdown
Contributor

@ievdokdm ievdokdm left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread app_dart/lib/src/model/firestore/ci_staging.dart Outdated
@jtmcdole jtmcdole added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App. and removed CICD Run CI/CD labels Mar 19, 2026
@auto-submit auto-submit Bot merged commit ca0c5ec into main Mar 19, 2026
20 checks passed
@jtmcdole jtmcdole deleted the mqgClosedOnSuppressed branch April 16, 2026 15:53
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.

2 participants