Skip to content

flutter_tools: do more refactor on CreateBase and CreateCommand#70959

Merged
fluttergithubbot merged 1 commit intoflutter:masterfrom
cyanglaz:create_refactor
Nov 20, 2020
Merged

flutter_tools: do more refactor on CreateBase and CreateCommand#70959
fluttergithubbot merged 1 commit intoflutter:masterfrom
cyanglaz:create_refactor

Conversation

@cyanglaz
Copy link
Copy Markdown
Contributor

@cyanglaz cyanglaz commented Nov 20, 2020 •
edited
Loading

Description

This PR includes additional refactoring to #70874

  1. Making projectDir and projectDirPath a getter in the CreateBase
  2. Making getFlutterRoot(), getProjectName () getters.
  3. Remove unnecessary parameters in validateProjectDir()

Related Issues

#59494

Tests

I added the following tests:

This is a pure refactoring, nothing new was added.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

@flutter-dashboard
Copy link
Copy Markdown

flutter-dashboard bot commented Nov 20, 2020

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Nov 20, 2020
@google-cla google-cla bot added the cla: yes label Nov 20, 2020
@cyanglaz cyanglaz requested a review from jmagman November 20, 2020 19:12
cyanglaz
cyanglaz commented Nov 20, 2020
View reviewed changes
packages/flutter_tools/lib/src/commands/create_base.dart
/// If `--org` is not specified, returns the organization from the existing project.
@protected
Future<String> getOrganization(Directory projectDir) async {
Future<String> getOrganization() async {
Copy link
Copy Markdown
Contributor Author

@cyanglaz cyanglaz Nov 20, 2020

Choose a reason for hiding this comment

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

@jmagman Since this method returns a future and it's not a constant lookup, I decided not making this a getter based on
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#getters-feel-faster-than-methods

jmagman
jmagman approved these changes Nov 20, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@fluttergithubbot
Copy link
Copy Markdown
Contributor

fluttergithubbot commented Nov 20, 2020

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot
Copy link
Copy Markdown
Contributor

fluttergithubbot commented Nov 20, 2020

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot merged commit ffe5197 into flutter:master Nov 20, 2020
@cyanglaz cyanglaz deleted the create_refactor branch November 21, 2020 01:36
@oshtman oshtman mentioned this pull request May 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jmagman jmagman jmagman approved these changes

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cyanglaz @fluttergithubbot @jmagman