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.

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

Adds uninstall page #10038

Merged
merged 7 commits into from
Jan 22, 2024
Merged

Adds uninstall page #10038

merged 7 commits into from
Jan 22, 2024

Conversation

atsansone
Copy link
Contributor

Add uninstall page. Fixes #5695

@flutter-website-bot
Copy link
Collaborator

flutter-website-bot commented Jan 12, 2024

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

https://flutter-docs-prod--pr10038-fix-5695-pn4828ey.web.app

{% else -%}
{% assign dirinstall='~/development' %}
{% assign dirconfig='~/' %}
{% assign path='~/development/' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% assign path='~/development/' %}
{% assign path='~/path/to/' %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment: This path would be consistent with the install path in the Install Guides. I could change it there, too. Can we mark this as an issue to resolve separately?

Copy link
Contributor

@johnpryan johnpryan Jan 16, 2024

Choose a reason for hiding this comment

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

Can you fix this as part of this PR? We shouldn't be specifying /development/ as the directory you install the SDK in. I install my in ~/code/sdks so if I followed these instructions I would be doing the wrong thing.

Copy link
Contributor

@sfshaza2 sfshaza2 Jan 16, 2024

Choose a reason for hiding this comment

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

Tony, please ignore my emoji's on your comment, but I see no way to delete them. I meant to thumbs up John's comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

(You can click the emojis again to delete them)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@johnpryan : In a guide, it helps to provide something. The line above the example states "This guide presumes that you installed Flutter in ~/development/ on ." I could add a line stating "or wherever you installed your Flutter SDK". That I could do without revisiting all of the example directories I set. That's what I would prefer.

Comment on lines 118 to 158
### Remove Dart configuration files
{:.no_toc}

If you don't want to preserve your Dart configuration,
remove the following directories from your home directory.

```terminal
{{ dart-files | strip}}
```

To remove these directories, run the following command.

```terminal
{{rm-dart-files | strip}}
```

### Remove pub package files
{:.no_toc}

{{site.alert.important}}
If you want to remove Flutter but not Dart,
don't complete this section.
{{site.alert.end}}

If you don't want to preserve your pub packages,
remove the `.pub-cache` directory from your home directory.

```terminal
{{rm-pub-dir | strip}}
```

</div>

{% endfor -%}
</div>
{% comment %} End: Tab panes. {% endcomment -%}
Copy link
Contributor

Choose a reason for hiding this comment

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

This section should live on dart.dev dart-lang/site-www#3187

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment: It can't just live there. The location of the configuration files is the same for both, but a complete removal should include all steps here.

Copy link
Contributor

@johnpryan johnpryan Jan 16, 2024

Choose a reason for hiding this comment

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

It can't just live there.

Yes it can and it should. We don't want duplicate docs between the sites.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, @johnpryan. We do want to avoid duplicatoin, so docs.flutter.dev should talk about removing Flutter. Then, if you want to remove Dart, go to dart.dev.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not about deleting Dart per se. It's about deleting Flutter. If you delete Flutter, you delete Dart at the same time, not at some other time. It doesn't make sense to send the user elsewhere to delete a config directory for Dart which is part of their complete Flutter install.

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.

Overall, this page looks good and I like the use of tabs. (But are they visible enough? My eyeballs slid over them on my first read of the staged version.)

  • Several times this PR uses the phrase "This guides presumes..." That struct me a bit odd, so I looked into using presumes vs assumes. The Google style guide doesn't seem to mention it (at least I couldn't find it), but many sources consider "presume", at least in some instances, rude. I looked at our website, and we use assumes 45 times, and presumes 3 times. I personally prefer "assumes" over "presumes", as the latter seems a bit off-putting, but this is a nit, as it's more of an issue of which is easier for non-native English speakers? If we do choose one over the other, we should list that in go/dart-terms
  • Can you add a friendly, "Come back anytime" sort of note at the end?
  • I assume you've had this tech reviewed, as I know Unix (so, by extension, macOS, Linux, and Chrome), but not Windows, so I have no idea about those commands.
  • This is a nit with the site rendering, not this PR specifically, but at first, I couldn't see the asterisks in the terminal commands. So, instead of seeing "rm -rf .dart*" I saw "rm -rf .dart". The copy button worked fine, of course, and I confirmed it by tanking my .dart files , but I wish that the text was more readable for the visually impaired (like me)
  • @parlough, can you weigh in on the location of this file in the site nav? I have sort of mixed feelings about it, as it's under "Getting Started", but I'm not sure if there's a better place.

@atsansone
Copy link
Contributor Author

atsansone commented Jan 16, 2024

@sfshaza2 : Good points.

  1. I had to check the presume vs. assume issue. Merriam-Webster clarifies in this way:

Although presume and assume both mean "to take something as true," "presume" implies more confidence or evidence backed reasoning. An "assumption" suggests there is little evidence supporting your guess. Think carefully before using them interchangeably or you may lose some meaning.

  1. Good point. I'll do that.
  2. I tested all of these commands myself on my laptop and my Windows box.
  3. I know, right? Same with semi-colons, which are used to put two commands on the same line. Maybe we can fix that.
  4. I'm not thrilled with it, but I didn't want to be accused of burying it, either!

@sfshaza2
Copy link
Contributor

Wow, those asterisks are MUCH more readable! thx

@parlough
Copy link
Member

@parlough, can you weigh in on the location of this file in the site nav? I have sort of mixed feelings about it, as it's under "Getting Started", but I'm not sure if there's a better place.

I'm not thrilled with it, but I didn't want to be accused of burying it, either!

I personally probably would have not added it to the sidenav at all, and instead link to it from some relevant pages. I can't imagine any user ever looking at the site sidenav to figure out how to uninstall Flutter.

If you think it deserves a spot in the sidenav though, I agree that it shouldn't be under "Get Started". I think another fine place would be under "Stay up to date" next to "Update" and "SDK archive" as that's the most relevant content we have.

@atsansone
Copy link
Contributor Author

atsansone commented Jan 17, 2024

@sfshaza2 , @parlough : Moved uninstall to under What's New on the left nav. PTAL.

@johnpryan
Copy link
Contributor

johnpryan commented Jan 17, 2024

I agree with @sfshaza2 and @parlough that that this link doesn't need to be in the sidenav, unless we have data that suggests a lot of visitors to our docs site are trying to find the uninstall link. Can we add a link to this page from the get-started/install instead? That would probably be the first place most users would look if they want to change things about their installation.

@atsansone
Copy link
Contributor Author

@johnpryan @sfshaza2 @parlough : In reading the original request, linking from Install and/or Update Flutter should be fine. This page needs information on how to remove Flutter from the PATH as well.

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.

So, @atsansone, why the empty https://github.com/flutter/website/pull/10038/files#diff-04964e3043b84ee2b1bbf612391351b1a20b8a156f1629d1c5d6a1d71ca21ee9 file?

I'm mostly ok with this PR, but I am not ok with having instructions for removing Dart on this site, but nothing on the Dart site. That portion should be moved out of this PR and into a PR for dart.dev. And then one can point to the other.

@atsansone
Copy link
Contributor Author

atsansone commented Jan 18, 2024

It's the unset path instructions for macOS. I will populate it when the other PR is merged.

There's already an issue on the Dart site that I'll do after this one.

Also: the instructions are not for removing Dart, but the Dart config files that get created when Flutter got installed. Flutter does include Dart in its install and Dart gets swept away when the Flutter directory is removed, but the config files remain.

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.

It occurs to me that there's some nuance here that needs to be covered.
If they've ONLY installed Flutter (and not Dart), then the Dart installation is underneath Flutter. So removing the Flutter directory ALSO removes the Dart directory. (They still need to remove it from their path, however.)

If they've installed Dart AS WELL as Flutter, then there are TWO installations of Dart. I assume that the PATH refers to the Dart that is inside the Flutter install. This should really be explained somewhere.

@atsansone atsansone added st.RFM Ready to merge or land and removed review.copy Awaiting Copy Review labels Jan 19, 2024
@atsansone
Copy link
Contributor Author

It occurs to me that there's some nuance here that needs to be covered. If they've ONLY installed Flutter (and not Dart), then the Dart installation is underneath Flutter. So removing the Flutter directory ALSO removes the Dart directory. (They still need to remove it from their path, however.)

If they've installed Dart AS WELL as Flutter, then there are TWO installations of Dart. I assume that the PATH refers to the Dart that is inside the Flutter install. This should really be explained somewhere.

The instructions cover removing the configuration files for Dart, which Flutter creates once installed. These instructions don't cover removing Dart itself.

image

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 1984d2a into flutter:main Jan 22, 2024
9 checks passed
atsansone added a commit to atsansone/website that referenced this pull request Jan 24, 2024
atsansone added a commit to atsansone/website that referenced this pull request Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
st.RFM Ready to merge or land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include instructions to uninstall Flutter
6 participants