Skip to content

Code Cleanup for Happy Devs#316

Open
mikewillems wants to merge 12 commits intostagingfrom
mw/debt/code-cleanup
Open

Code Cleanup for Happy Devs#316
mikewillems wants to merge 12 commits intostagingfrom
mw/debt/code-cleanup

Conversation

@mikewillems
Copy link
Collaborator

@mikewillems mikewillems commented Mar 2, 2026

What is in this PR?

Clean it up! Fix the stagnant analyzer warnings (and INFO level alerts in the touched files - but not in other files). This means that:

  • We get to have a nice unhighlighted repo (with the current analyzer options), and that every time a file gets highlighted, it actually means something and should be addressed.
  • We don't have to have a bunch of untracked generated files polluting our git status all the time.

Specifically, fixed all 56 WARNING-level issues:

32× unused_import
9× dead_code
5× unused_local_variable
3× override_on_non_overriding_member
2× unused_field
2× unused_result
1× dead_null_aware_expression
1× duplicate_import
1× sdk_version_since

Changes in the codebase

Branch: mw/debt/code-cleanup (diverged from staging at 55024b26)


Part 1: Commit Summary

# Commit Summary
1 a379c0b2 Formatting only. Whitespace / trailing-comma / line-wrap corrections in 5 files so subsequent diffs are cleaner.
2 d755ad62 Removed unused imports & variables across 25 client files.
3 1d297b30 Registered implicit deps - added collection (client) and meta (functions) to pubspec.yaml.
4 298a1968 SDK version constraints - updated both client & functions sdk: '>=3.0.0 <4.0.0'.
5 3ab49d3a Removed one more unused import (localization_helper.dart in event_info.dart).
6 953bf26a Removed dead screenshare code - 45-line block in control_bar.dart. (#47)
7 f2b1ea9a Removed unreachable code & unnecessary conditionals in 6 files (client + functions). Includes dead showTime helper, unreachable return null, dead mobile-page builder, unnecessary ?? false.
8 4dd6a15f Removed unused showChatOption toggle & UI in waiting room widget. (#315)
9 fff73422 Follow-up for showChatOption - removed enableChat field from WaitingRoomInfo model, serialization, presenter method. (#315)
10 c1c412b9 Bugfix: assign copyWith results in PrePostCardWidgetPresenter - two calls were silently discarding mutations.
11 6bb7869d Documentation - updated docs/pages/dev.md with instructions for ignoring system-specific untracked files.

Changes outside the codebase

None.

Testing this PR

There should be no observable differences in the app - only one bug fix and it was mostly silent. Nothing to test.

Additional information

A bunch of the issues addressed here are present elsewhere in the code base but currently silent due to the analyzer settings. We can fix a lot of problems for almost free if we check for these. I categorized all the "PROBLEMS" still found, all currently INFO level because that's the default.

Remaining Easy-fix Problems

This is the full list, grouped by the level I think we should assign to them:

Elevate to error (bugs that are probably already biting us):

  • use_build_context_synchronously - Using a BuildContext after an async gap can crash or operate on a disposed widget. Probably several of our sentry errors, and already fixed some.

Elevate to warning so we see them in the file explorer:

  • unused_element - dead code just like unused_import (which is currently a warning).
  • avoid_print - shouldn't ship prints in prod.
  • implementation_imports - importing from src/ breaks without warning on package updates.
  • depend_on_referenced_packages - using transitive dependency without declaring it, breaks if intermediary drops it.
  • require_trailing_commas - formatting preference, but since our formatter will already catch it and modify files accordingly on save, I think we proactively fix it.

Leave as INFO for now, refactor later:

  • deprecated_member_use / deprecated_member_use_from_same_package - just helpful breadcrumbs to remind us to update calls after framework updates.
  • library_private_types_in_public_api - style issue only and fine / super common in Flutter
  • constant_identifier_names - style only.
  • unnecessary_cast / prefer_const_declarations - minor.

Proposed analyzer changes:

errors:
    # Bugs
    use_build_context_synchronously: error
    unawaited_futures: error
    unnecessary_null_comparison: error
    missing_required_param: error

    # Should be visible / fixed promptly
    unused_import: warning
    unused_element: warning
    dead_null_aware_expression: warning
    avoid_print: warning
    implementation_imports: warning
    depend_on_referenced_packages: warning
    require_trailing_commas: warning

    # Freezed recommendation
    invalid_annotation_target: ignore

Purpose is that all subsequent changes will show only the actual
material changes and be easier to gauge.
both to be realistic (nothing we're using would ever touch <3.0.0)
updateAttributeTypeSelection and updateEnteredQueryName were calling
_model.prePostCard.copyWith() without assigning the result, silently
discarding changes to attribute type and query param name. The model
was left stale and changes would be lost on any save/reload cycle.
@mikewillems mikewillems changed the title Mw/debt/code cleanup Code Cleanup for Happy Devs Mar 2, 2026
@mikewillems mikewillems marked this pull request as draft March 2, 2026 19:31
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

Visit the preview URL for this PR (updated for commit a13ed46):

https://gen-hls-bkc-7627--pr316-mw-debt-code-cleanup-bfwcc2dc.web.app

(expires Mon, 16 Mar 2026 19:41:51 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: eed668cca81618d491d024574a8f8a6003deaa8d

@mikewillems mikewillems marked this pull request as ready for review March 2, 2026 19:45
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