Skip to content

Conversation

carlosms
Copy link
Contributor

@carlosms carlosms commented Feb 28, 2018

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

@carlosms carlosms requested a review from bzz February 28, 2018 10:40
Copy link
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.

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
Contributor

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
Contributor

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
Contributor Author

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

@bzz
Copy link
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
Contributor

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

@carlosms
Copy link
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)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants