[go: nahoru, domu]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes PATH variance, filename replacement script #10706

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Fixes PATH variance, filename replacement script #10706

merged 1 commit into from
Jun 6, 2024

Conversation

atsansone
Copy link
Contributor

Fixes #10662

@atsansone atsansone requested a review from sfshaza2 as a code owner June 4, 2024 21:24
@atsansone atsansone added the review.tech Awaiting Technical Review label Jun 4, 2024
@flutter-website-bot
Copy link
Collaborator

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

https://flutter-docs-prod--pr10706-fix-10662-wi3so5uu.web.app

sfshaza2
sfshaza2 reviewed Jun 5, 2024
View reviewed changes
Copy link
Contributor
@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

Not a full tech review, just questions.

src/_data/shells.yml
@@ -1,14 +1,14 @@
- name: bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Might it be that Flutter is NOT in /usr/bin?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but this is consistent with the previous advice given on the page, which is to consider adding flutter to /usr/bin. If they follow inline, this should be OK.

src/_includes/docs/install/flutter/download.md
@@ -8,17 +8,17 @@
{% assign path='C:\\user\\{username}\\dev' %}
{% assign flutter-path='C:\\user\\{username}\\dev\\flutter' %}
{% assign terminal='PowerShell' %}
{% assign prompt='C:>' %}
{% assign prompt='PS C:\\>' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we weren't using :PS C"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We encourage PowerShell everywhere, and the PowerShell prompt is PS C:\\>. I noticed it on my PC the other day and wanted to be consistent / correct.

@atsansone
Copy link
Contributor Author

@sfshaza2 : I'll hold off merging if you have anything else. I'd like to land it soon, though, because there's a related issue (#10696) that I'll need to change this same section. (It's waiting on validation from the Android Studio team.)

sfshaza2
sfshaza2 approved these changes Jun 6, 2024
View reviewed changes
Copy link
Contributor
@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

lgtm

@sfshaza2 sfshaza2 merged commit 0799166 into flutter:main Jun 6, 2024
9 checks passed
@sfshaza2 sfshaza2 removed the review.tech Awaiting Technical Review label Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@domesticmouse domesticmouse domesticmouse approved these changes

@sfshaza2 sfshaza2 sfshaza2 approved these changes

@parlough parlough Awaiting requested review from parlough parlough is a code owner

@MaryaBelanger MaryaBelanger Awaiting requested review from MaryaBelanger MaryaBelanger is a code owner

Assignees

@sfshaza2 sfshaza2

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Resolve difference between install and PATH instructions for Linux
4 participants
@atsansone @flutter-website-bot @domesticmouse @sfshaza2