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.

Sign up for GitHub

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

Jump to bottom

Update the README.md #6

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Update the README.md #6

wants to merge 4 commits into from

Conversation

waleCloud
Copy link

Documentation done with Markdown.

The description was left out blank.
kindly drop the description of the package here and i will update it as soon as possible.

@waleCloud
Update the README.md
9182c06 
Documentation done with Markdown
joestrong
joestrong requested changes Oct 12, 2017
View reviewed changes
Copy link
Owner

@joestrong joestrong left a comment

Choose a reason for hiding this comment

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

A few things I think should be adjusted

README.md Outdated
MIT


**Free Software, Hell Yeah!**
Copy link
Owner

Choose a reason for hiding this comment

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

Remove this

README.md Outdated



### Todos
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the 'todos' section

README.md Outdated

```php
Auth with the API
Copy link
Owner

Choose a reason for hiding this comment

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

Make the descriptions in the PHP block comments

README.md Outdated

`composer require joestrong/retroachievements`
- RetroAchievements composer package is completely open sourced. Any [contribution](https://github.com/joestrong/retroachievements-composer/#contribute) is highly appreciated.
Copy link
Owner

Choose a reason for hiding this comment

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

May as well remove the contribute link until there are contribution guidelines

joestrong
joestrong requested changes Oct 12, 2017
View reviewed changes
README.md Outdated

Install with:
RetroAchievements composer Package is a ......................................................
Copy link
Owner

Choose a reason for hiding this comment

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

For the description:
"RetroAchievements composer Package is a simple wrapper around the RetroAchievements.org API written in PHP"

joestrong
joestrong requested changes Oct 12, 2017
View reviewed changes
README.md Outdated
@@ -1,12 +1,26 @@
## RetroAchievements composer package
[![N|Solid](https://encrypted-tbn0.gstatic.com/images?q=tbn:ANd9GcRAhk6glux07Qv-MJHXlBmK8LQ0gKALFPp2Yc_UTMGd9AMR_gRLKg)]()
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove the composer image?

README.md Outdated

# Installation

RetroAchievements composer package requires [Composer](https://composer.org/) to run.
Copy link
Owner

Choose a reason for hiding this comment

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

Can your remove this sentence

README.md Outdated

RetroAchievements composer package requires [Composer](https://composer.org/) to run.

#### Install the dependencies.
Copy link
Owner

Choose a reason for hiding this comment

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

Can your remove this sentence (this is the package to install, it's not its own dependency)

@waleCloud
Updated REAME.md
a05e8c3 
Reviews attended to. 
Kindly validate
@waleCloud
Copy link
Author

Done!

@joestrong joestrong changed the base branch from master to develop October 12, 2017 21:35
@joestrong joestrong changed the base branch from develop to master October 12, 2017 21:37
joestrong
joestrong requested changes Oct 12, 2017
View reviewed changes
Copy link
Owner

@joestrong joestrong left a comment

Choose a reason for hiding this comment

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

Can you address the 2 changes I've suggested.
Also:

  • Under the 'Methods' heading, the text above each bit of code is being interpreted in the php block. Can the text be either comments in the php block, or split up the code block?
  • You can re-add the 'Licence' heading, with just 'MIT'

README.md Outdated
# RetroAchievements composer package
```sh
/* RetroAchievements composer Package is a simple wrapper around the RetroAchievements.org API written in PHP
Copy link
Owner

Choose a reason for hiding this comment

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

This should be just a regular sentence (no bash syntax/comment)

README.md Outdated
# Open Source

- RetroAchievements composer package is completely open sourced. Any [contribution](https://github.com/joestrong/retroachievements-composer/#contribute) is highly appreciated.
- RetroAchievements composer package is completely open sourced.
Copy link
Owner

Choose a reason for hiding this comment

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

Remove the bullet point

@waleCloud
Copy link
Author

Suggested changes made and MIT License re-added.

joestrong
joestrong requested changes Oct 13, 2017
View reviewed changes
Copy link
Owner

@joestrong joestrong left a comment

Choose a reason for hiding this comment

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

Can you amend the description?

Then once that's done, are you able to rebase the changes on to the latest version of the develop branch?

README.md Outdated
@@ -1,12 +1,22 @@
## RetroAchievements composer package
# RetroAchievements composer package
```sh
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove the bash markup around the description?

@joestrong
Copy link
Owner

Can you pull the latest develop branch and rebase your changes onto there?

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

@joestrong joestrong joestrong approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@waleCloud @joestrong