Skip to content

Changing sudo pip to pip in readme.rst file.#4224

Closed
kshithijiyer wants to merge 2 commits intoaws:developfrom
kshithijiyer:patch-1
Closed

Changing sudo pip to pip in readme.rst file.#4224
kshithijiyer wants to merge 2 commits intoaws:developfrom
kshithijiyer:patch-1

Conversation

@kshithijiyer
Copy link
Copy Markdown

fixes: #4175

@kshithijiyer kshithijiyer changed the title Changing sudo pip to pip readme.rst file. Changing sudo pip to pip in readme.rst file. Jun 7, 2019
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jun 7, 2019

Codecov Report

Merging #4224 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4224   +/-   ##
========================================
  Coverage    94.41%   94.41%           
========================================
  Files          188      188           
  Lines        14151    14151           
========================================
  Hits         13360    13360           
  Misses         791      791

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 069c7f9...3b083ef. Read the comment docs.

or, if you are not installing in a ``virtualenv``, to install globally::

$ sudo pip install awscli
$ pip install awscli
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This probably won’t work, plus it’s not right in this part of the doc (it’s the same as the command just before).

pip install --user awscli could be an alternative that works without virtual env and without messing us the system (or being denied)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

So should we just remove it or leave it as it is?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don’t understand the question.

sudo pip install is bad, pip install won’t work. More thought is needed (e.g. with the install --user idea)

@kdaily
Copy link
Copy Markdown
Member

kdaily commented Jun 23, 2022

Hi @kshithijiyer,

Apologies for the delay. We're cleaning up our backlog according to our new contribution process. I understand the sentiment of this change, and agree with @merwok that just dropping the sudo is probably not the best. I would possibly just drop this command entirely and rely on suggesting that users use a virtual environment. The logic here is that's what the AWS CLI v1 installation guide also suggests that, as well as a --user install. It does not direct users to install system-wide without using an operating system's package manager.

I think we should draw a better distinction between installing for development purposes here in the GitHub repository, and direct end users more towards the content in the Installation section of the AWS CLI User Guide.

Given the lag between opening this and now, I'm happy to make these changes, but I can leave it for you to do as well since it's a minimal change from what you already have. It might be better for you to open a new PR given the conflicts present from the long time; otherwise you'll need to rebase as well.

Let me know how you'd like to proceed!

@kdaily kdaily added documentation This is a problem with documentation. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. installation labels Jun 23, 2022
@kdaily kdaily self-assigned this Jun 23, 2022
@kdaily kdaily added the feature-request A feature should be added or improved. label Jun 23, 2022
@kdaily
Copy link
Copy Markdown
Member

kdaily commented Jun 23, 2022

After some further discussion, we'd be even happier to remove the entire installation section, save for things that a developer would use, and redirect all end users to the User Guide installation section.

@tim-finnigan
Copy link
Copy Markdown
Contributor

Given that we haven't heard back on this PR, I'm going to close it due to inactivity. As suggested above, there are broader improvements to the installation documentation that could be made here and I think that can be in a separate PR.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.41%. Comparing base (069c7f9) to head (3b083ef).
⚠️ Report is 6217 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4224   +/-   ##
========================================
  Coverage    94.41%   94.41%           
========================================
  Files          188      188           
  Lines        14151    14151           
========================================
  Hits         13360    13360           
  Misses         791      791           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

documentation This is a problem with documentation. feature-request A feature should be added or improved. installation response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Do not recommend sudo pip

6 participants