这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@jason-simmons
Copy link
Member

No description provided.

…command instead of the main source file path
@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Aug 1, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly changes the suggested cd command in flutter create to use the project directory path instead of the main source file path. The accompanying test change is also an improvement. I've suggested a small refinement to the test to make it more robust.

Comment on lines 4822 to 4823
RegExp cdLine = RegExp(r'^ \$ cd .*flutter_project$', multiLine: true);
expect(cdLine.hasMatch(logger.statusText), isTrue);
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This is a good improvement to make the test more specific. To make it even more robust and less reliant on hardcoded strings, you could construct the expected path from the projectDir variable. This avoids hardcoding flutter_project and ensures the test correctly handles any changes to the test setup or relative path generation.1

    final String expectedPath = globals.fs.path.relative(projectDir.path);
    final RegExp cdLine = RegExp(r'^  \$ cd ' + RegExp.escape(expectedPath) + r'$', multiLine: true);
    expect(cdLine.hasMatch(logger.statusText), isTrue, reason: 'statusText does not contain "cd $expectedPath"');

Style Guide References

Footnotes

  1. Code should be tested and follow the guidance described in the writing effective tests guide. (link)

@matanlurey
Copy link
Contributor

If this made it into 3.35 we should probably cherrypick it (to beta) after merging.

@jason-simmons
Copy link
Member Author

This issue does not affect the 3.35 branch.

@jason-simmons jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 4, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 4, 2025
Merged via the queue into flutter:master with commit 40fcfc7 Aug 4, 2025
141 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 4, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 5, 2025
danilozhang pushed a commit to danilozhang/flutter that referenced this pull request Aug 6, 2025
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
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.

2 participants