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

Improve documentation #199

Merged
merged 4 commits into from
Nov 14, 2017
Merged

Improve documentation #199

merged 4 commits into from
Nov 14, 2017

Conversation

agisilaos
Copy link
Contributor

@agisilaos agisilaos commented Aug 26, 2017

This PR contains improvements to the current documentation. Before explaining the changes I would like to take a moment & explain why I made the PR.

So initially I had some problems getting started with chisel & installing it I thought I was the only one. But it turned out that other people as well had problems finding .lldbinit file, or even create it (A couple of issues created with such problems from users).

These changes aim to bring clarity to the docs & help people that are new with such files/ installing projects like that get started in a few minutes. Into a bit more detail:

  1. Added commands so the user can check if .lldbinit file exists & if it doesn't exist, to create it.

  2. Added the command that needs to be added on the .lldbinit file when someone installs chisel with brew & I made such changes because sometimes user will not pay attention to the terminal after installing chisel with brew so now the docs will be improved & the experience of the user when he will install chisel much better.

These changes are one part of the overall adjustments/improvements that I'm thinking of doing on the documentation side of things. I would like to thank you for creating such an awesome tool for iOS engineers to work better while debugging.

Copy link
Contributor

@kastiglione kastiglione left a comment

Choose a reason for hiding this comment

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

Thanks for this, and for the quality explanation.

I think the instructions could be simplified by dropping the ls and instead advise everyone run touch then open.

README.md Outdated

```shell
touch .lldbinit //creates file
open .lldbinit //opens file
Copy link
Contributor

Choose a reason for hiding this comment

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

If someone doesn't know these commands, they might also not realize that the // comments are not supposed to be written. Generally I prefer not adding comments at the end of command lines, but if you feel strongly then they should at least be commented with # which is the shell syntax for comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oups my bad there! Yeah makes sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the commands are self explanatory so I dropped the comments there. Good point

@agisilaos
Copy link
Contributor Author

agisilaos commented Aug 27, 2017

I think I covered all of your points/ mentions @kastiglione. Can you please take a look & tell me what you think? 😃

@agisilaos
Copy link
Contributor Author

Hey @kastiglione, any updates on this Pull Request? Is it ready to be merged? What do you think? Are there any other changes that you may want to suggest?

@kastiglione kastiglione merged commit 1b3e34d into facebook:master Nov 14, 2017
@kastiglione
Copy link
Contributor

Thanks for contributing this @agisilaos. Apologies for not being more of a prompt reviewer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants