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

Update ReadMe #213

Closed
wants to merge 2 commits into from
Closed

Update ReadMe #213

wants to merge 2 commits into from

Conversation

DDeAlmeida
Copy link

Hi, after using ReadMe to manage the Guildnet explorer Indexer, i saw some modification required.
@frol if you agree with this modifications ;)

Comment on lines +33 to +39
* Cargo dependency
*
On Debian/Ubuntu:

```bash
sudo apt install cargo
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Installing Rust compiler is the first item on the list. Rustup is more flexible way to install and manage rustc/cargo versions

Copy link
Author

Choose a reason for hiding this comment

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

Yep but Cargo is mandatory for install diesel_cli
So he must be added to the list

Copy link
Contributor

Choose a reason for hiding this comment

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

cargo gets installed when you install Rustup. It is really hard to install Rust without cargo. Thus, there is no need for this extra step.

@@ -28,52 +28,70 @@ Before you proceed, make sure you have the following software installed:
On Debian/Ubuntu:

```bash
$ sudo apt install libpq-dev
sudo apt install libpq-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it is a commit practice to use $ to indicate that the command should be run on behalf of a regular user.

Copy link
Author

Choose a reason for hiding this comment

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

Oh ok, but this is a pain when copy/paste.
Sorry for this commit.

Comment on lines +43 to +55
Setup PostgreSQL database

### Prepare Database
```bash
sudo apt-get install postgresql postgresql-contrib
```

Setup PostgreSQL database, create a database with the regular tools, and note the connection string (database host, credentials, and the database name).
Create a database with the regular tools (Change user and db_name with yours)
```bash
sudo su postgres
psql
create database db_name with owner=user;
```
Note the connection string (database host, credentials, and the database name).
Copy link
Contributor

Choose a reason for hiding this comment

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

Meh, setting up a database is a complicated topic, so I would avoid giving any instructions. Consider finding a good article out there and link it from this README

Copy link
Author

Choose a reason for hiding this comment

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

Or we can just give it as a example. as you want

```

And apply migrations

```bash
$ diesel migration run
diesel migration run
Copy link
Member

Choose a reason for hiding this comment

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

Even after this command?

$ cargo install diesel_cli --no-default-features --features "postgres"

@frol frol closed this Apr 5, 2022
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