Skip to content

firebase-get-to-know-flutter: separate main.dart into different files#1206

Merged
domesticmouse merged 8 commits into
flutter:mainfrom
miquelbeltran:mb-firebase-get-to-know-modules
Dec 8, 2022
Merged

firebase-get-to-know-flutter: separate main.dart into different files#1206
domesticmouse merged 8 commits into
flutter:mainfrom
miquelbeltran:mb-firebase-get-to-know-modules

Conversation

@miquelbeltran
Copy link
Copy Markdown
Member

@miquelbeltran miquelbeltran commented Dec 6, 2022

As preparation for migrating the codelab to go_router (and Material 3 later on)

Work is mostly copy/paste and re-formatting, changes are organized in commit per step for easier review.

Final code organization is as follows:

image

The only doubt I had was if the Attending enum should be a separated file or not, since it was only used by the ApplicationState, I left it inside app_state.dart

I did not update the codelab_rebuild.yaml as I am still not sure how to do that, but I can give it a try if you let me know how.

We would need to update the codelab document as well. I can do that after go_router has been adopted, or I can do that now before migrating to go_router. Please let me know what you prefer.

cc. @domesticmouse @nohe427 @redbrogdon

Pre-launch Checklist

  • I read the Effective Dart: Style recently, and have followed its advice.
  • I signed the CLA.
  • I updated/added relevant documentation (doc comments with ///). <-- Codelab needs update
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-devrel channel on Discord.

@miquelbeltran
Copy link
Copy Markdown
Member Author

miquelbeltran commented Dec 6, 2022

I realized that the files in this codelab don't have a copyright header, which is present in other codelabs. Let me know if the new files need that as well.

@nohe427
Copy link
Copy Markdown
Contributor

nohe427 commented Dec 6, 2022

Yes, @miquelbeltran - Please add license headers

@domesticmouse
Copy link
Copy Markdown
Contributor

I realized that the files in this codelab don't have a copyright header, which is present in other codelabs. Let me know if the new files need that as well.

I have a WIP PR that is going to be adding license headers per https://github.com/flutter/codelabs/blob/main/CONTRIBUTING.md#file-headers

I've yet to make it as far as this codelab tho =)

@nohe427
Copy link
Copy Markdown
Contributor

nohe427 commented Dec 6, 2022

I think the codelab.md might be out of sync with what is published on the site. I can land an update to that momentarily.

Copy link
Copy Markdown
Contributor

@nohe427 nohe427 left a comment

Choose a reason for hiding this comment

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

LGTM

@miquelbeltran
Copy link
Copy Markdown
Member Author

Added license headers. I skipped the firebase_options.dart file since it is generated by the FlutterFire CLI.

Copy link
Copy Markdown
Contributor

@domesticmouse domesticmouse left a comment

Choose a reason for hiding this comment

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

Free to land when tests pass

@domesticmouse domesticmouse merged commit cc703b0 into flutter:main Dec 8, 2022
@miquelbeltran miquelbeltran deleted the mb-firebase-get-to-know-modules branch December 11, 2022 08:33
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.

3 participants