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

Added configure section with note for Docker users #2141

Merged
merged 3 commits into from
Apr 26, 2017
Merged

Added configure section with note for Docker users #2141

merged 3 commits into from
Apr 26, 2017

Conversation

marcoliceti
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Apr 26, 2017

Codecov Report

Merging #2141 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2141   +/-   ##
======================================
  Coverage    8.89%   8.89%           
======================================
  Files          92      92           
  Lines        6837    6837           
======================================
  Hits          608     608           
  Misses       6094    6094           
  Partials      135     135

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 61e511c...34ce1e7. Read the comment docs.

@harshavardhana
Copy link
Member

Thanks for the PR @marcoliceti - reviewing now.

README.md Outdated
@@ -34,6 +34,8 @@ docker pull minio/mc:edge
docker run minio/mc:edge ls play
```

**Note:** These examples refer to the _play_ environment, a public Minio server for testing pruposes. If you want to try the same commands against a different environment, check [Configure](#configure).
Copy link
Member

Choose a reason for hiding this comment

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

Typo s/pruposes/purposes/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

README.md Outdated
@@ -95,6 +97,30 @@ If you do not have a working Golang environment, please follow [How to install G
go get -u github.com/minio/mc
```

## Configure

By default `mc` is configured to interact with the _play_ environment, a public Minio server. If you want to configure a different server:
Copy link
Member

Choose a reason for hiding this comment

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

If you want to configure a different S3 compatible server:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

README.md Outdated
```

You will be able to list buckets (i.e. `mc ls <ALIAS>`) or any other available operation.

Copy link
Member

Choose a reason for hiding this comment

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

BTW this looks like a repeated mention.. i.e there are details about adding S3 and GCS below, we don't need to repeat the #Configure section.

Perhaps you can move the #Docker section after # Test your setup section underneath.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed duplicated info

README.md Outdated

### Endpoint and credentials

At startup, Minio server provides its endpoint, access key and secret key.
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to talk about Endpoint and credentials from just Minio server point of view. mc is a generic tool to be used with all S3 compatible servers, we can move this to # Example - Minio Cloud Storage section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

@harshavardhana harshavardhana requested a review from nitisht April 26, 2017 09:42
README.md Outdated
@@ -34,6 +34,14 @@ docker pull minio/mc:edge
docker run minio/mc:edge ls play
```

**Note:** These examples refer to the [_play_ environment](#test-your-setup). If you want to try the same commands against a different S3 compatible server, start the container this way:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about - Above examples run mc against Minio play environment by default. To run mc against other S3 compatible servers, start the container this way:

README.md Outdated
```

then use the `mc config` command to [configure `mc`](#add-a-cloud-storage-service).

Copy link
Contributor

Choose a reason for hiding this comment

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

then use the mc config command.

@marcoliceti
Copy link
Contributor Author

@nitisht I updated the README with your suggestions.

@nitisht
Copy link
Contributor

nitisht commented Apr 26, 2017

Thank you for the contribution @marcoliceti

@harshavardhana harshavardhana merged commit 39aa635 into minio:master Apr 26, 2017
@marcoliceti marcoliceti deleted the patch-1 branch April 27, 2017 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants