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
Contributor

@cyanglaz cyanglaz commented Nov 20, 2020

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

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
/// 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
Contributor Author

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

Copy link
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
Contributor

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
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants