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

Fixes #73: Updated README and CONTRIBUTING #132

Closed
wants to merge 1 commit into from

Conversation

cindyledev
Copy link
Contributor

Updated README.md and CONTRIBUTING.md with

  • License Section
  • Git as a prereq for contributing
  • Solution to a common Redis error on Linux

Ref #73 . Do not close this issue as @MusaBajwa is also assigned to it.

@Reza-Rajabi Reza-Rajabi added this to In progress/Review in Main via automation Nov 11, 2019
@humphd
Copy link
Contributor

humphd commented Nov 11, 2019

This has too many unrelated commits in here, I assume from a merge? Please rebase PRs instead of merging master into them.

@cindyledev
Copy link
Contributor Author

I got rid of the unrelated commits

@manekenpix manekenpix added the type: documentation (docs) Improvements or additions to documentation label Nov 13, 2019
```
# Could not create server TCP listening socket *:6379: bind: Address already in use
```
1. Run `sudo systemctl status redis` to check if the service is running
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unnecessary. Regular users should not have to access systemctl in this scenario. We are creating a docker container so errors addressed in this section should pertain to those that could be had from running redis in a docker container. Please read #135 for more information.

user 2570 0.0 0.0 253100 4908 pts/1 Sl+ 13:11 0:00 redis-server *:6379
user 2588 0.0 0.0 215744 888 pts/0 S+ 13:16 0:00 grep --color=auto redis
```
Run `sudo kill -9 2570`
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, this is unnecessary. See my comment above. FYI the process ID that you have added will likely not be the same for others running the same process. Suggesting an uninformed user to execute kill -9 with sudo on a specific pid like you are suggesting here is dangerous and could cause some major issues on their machine.

@cindyledev cindyledev closed this Nov 14, 2019
Main automation moved this from In progress/Review to Closed Nov 14, 2019
@cindyledev cindyledev deleted the issue-73 branch November 14, 2019 15:08
@seanprashad
Copy link
Contributor

Hey @cindyledev - there was some interesting work here and I'm sad to see that you closed your patch 😔I'd like to pair and continue improving the contributing docs, if you're up for it! Feel free to message me on slack - my username is sprashad 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation (docs) Improvements or additions to documentation
Projects
No open projects
Main
Closed
Development

Successfully merging this pull request may close these issues.

None yet

7 participants