Skip to content

Documentation improvements#177

Merged
carlosms merged 1 commit intosrc-d:masterfrom
carlosms:issue-67
Mar 1, 2018
Merged

Documentation improvements#177
carlosms merged 1 commit intosrc-d:masterfrom
carlosms:issue-67

Conversation

@carlosms
Copy link
Copy Markdown
Contributor

@carlosms carlosms commented Feb 28, 2018 •
edited
Loading

Based on #170.
The bullets from #67 fixed in this PR are:

@carlosms carlosms requested a review from bzz February 28, 2018 10:40
dpordomingo
dpordomingo approved these changes Feb 28, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@dpordomingo dpordomingo left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks for splitting the commits and for making the PR as small as possible.

smacker
smacker reviewed Feb 28, 2018
View reviewed changes
README.md Outdated
## Contributing

[Contributions](https://github.com/src-d/code-annotation/issues) are more than welcome, if you are interested please take a look to our [Contributing Guidelines](CONTRIBUTING.md).
[Contributions](https://github.com/src-d/code-annotation/issues) are more than welcome, if you are interested please take a look to our [Contributing Guidelines](CONTRIBUTING.md). You have more information on how to deploy it for [development purposes here](CONTRIBUTING.md#Development).
Copy link
Copy Markdown
Contributor

@smacker smacker Feb 28, 2018

Choose a reason for hiding this comment

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

@carlosms your English is better than mine. Are you sure deploy is the correct word here? For developers, we give instructions how to run locally not deploy. But I can be wrong.

Copy link
Copy Markdown
Contributor

@bzz bzz Feb 28, 2018

Choose a reason for hiding this comment

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

how to deploy it for ... -> how to build and run locally for ... ?

Copy link
Copy Markdown
Contributor Author

@carlosms carlosms Feb 28, 2018

Choose a reason for hiding this comment

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

You are right, running locally is more clear. Fixed in 1bde8b9

@smacker smacker mentioned this pull request Feb 28, 2018
Open
6 tasks
@bzz
Copy link
Copy Markdown
Contributor

bzz commented Mar 1, 2018

@carlosms could you please rebase, as #170 is merged now?

Might be also worth incorporating latest discussion from #67 before merging.

@dpordomingo
Copy link
Copy Markdown
Contributor

dpordomingo commented Mar 1, 2018

@carlosms now all the feedback seems to be addressed, can you merge?
Many thanks!

@carlosms
Copy link
Copy Markdown
Contributor Author

carlosms commented Mar 1, 2018

@dpordomingo I'm waiting because of the conversation on the issue, we may want to add more things to this PR: #67 (comment)

@carlosms
Update README with small improvements
f387491 
Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>

Add link to contributing.md development

Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>

Update readme screenshot

Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>

Improve wording for dev: deploy -> run locally

Signed-off-by: Carlos Martín <carlos.martin.sanchez@gmail.com>
@carlosms carlosms merged commit c2b995f into src-d:master Mar 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@smacker smacker smacker left review comments

@dpordomingo dpordomingo dpordomingo approved these changes

@bzz bzz bzz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@carlosms @bzz @dpordomingo @smacker