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

added idea (goland) to gitignore and updated documentation #242 #243

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

Conversation

liron-navon
Copy link

@liron-navon liron-navon commented Mar 19, 2018

Changes made:

On README.md

  1. Changed the examples syntax, kind of more welcoming to call variables tweet and values instead of t and v. "your-access-token" and "your-access-token-secret" are actually the user's token and secret, easier to understand this way if you are new to twitter's api.
  2. Added documentation for the filter streaming service and upload files.
  3. added link to the GoDoc
  4. added links to the appropriate twitter docs for each endpoint used in the readme to make it easier to understand and use.

On .gitignore
5. added .idea to gitignore (file created by Intelij idea and Goland ide's)

@muesli
Copy link
Collaborator

muesli commented Mar 20, 2018

@liron-navon: First of all, thanks for your PR!

There are a few issues with it however:

  1. Please group changes into individual commits. For example the documentation changes and .gitignore changes don't belong together and should be committed separately.
  2. Similarly, please open individual pull requests for changes that don't belong together.
  3. I don't think we should start adding IDE-specific files to .gitignore. You can add those to your personal, global gitignore file.
  4. You seem to have added a bunch of unnecessary whitespace to the README. Did this happen by accident? I'm not sure it makes the README more readable or less.

Please don't let this feedback discourage you, I really appreciate all the documentation updates!

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.

2 participants