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

Update docs v2 #427

Merged
merged 8 commits into from
Jul 15, 2019
Merged

Update docs v2 #427

merged 8 commits into from
Jul 15, 2019

Conversation

jimray
Copy link
Contributor

@jimray jimray commented May 24, 2019

Summary

I updated the docs to be compatible with v2 of the SDK. Almost all of the prose has been edited, updated, or entirely rewritten. Headlines clarified and navigation should be easier to follow. All of the sample code is updated for Py3 and v2 of our SDK.

I wasn't able to update the RTM docs yet.

Requirements (place an x in each [ ])

@CLAassistant
Copy link

CLAassistant commented May 24, 2019

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented May 24, 2019

Codecov Report

Merging #427 into update_docs_v2 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##           update_docs_v2     #427   +/-   ##
===============================================
  Coverage           56.96%   56.96%           
===============================================
  Files                   6        6           
  Lines                 732      732           
  Branches               42       42           
===============================================
  Hits                  417      417           
  Misses                306      306           
  Partials                9        9

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b95bce0...b940063. Read the comment docs.

Copy link
Contributor

@RodneyU215 RodneyU215 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 update @jimray! I've added a few comments inline. I also have a couple questions:

  1. Should we keep the v2 docs around for a few weeks so that people who are using those links aren't left with a 404?
  2. I'd like to move to markdown files instead of RestructuredText. Should we do that now or in a subsequent PR?

As mentioned above, we're setting the app tokens and other configs in environment variables and pulling them into global variables.

Depending on what actions your app will need to perform, you'll need different OAuth permission scopes. Review the available scopes `here <https://api.slack.com/docs/oauth-scopes>`_.
To configure your app for OAuth, you'll need a client ID, a client secret, and a set of one or more scopes that will be applied to the token once it is granted. The client ID and client secret are available from your `app's configuration page <https://api.slack.com/apps>`_. The scopes are determined by the functionality of the app -- every method you wish to access has a corresponding scope and your app will need to request that scope in order to be able to access the method. Review Slack's `full list of OAuth scopes <https://api.slack.com/docs/oauth-scopes>`_.

.. code-block:: python

Copy link
Contributor

Choose a reason for hiding this comment

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

The import statement here needs to be updated.


Once your user has completed the OAuth flow, you'll be able to use the provided
tokens to make a variety of Web API calls on behalf of the user and your app's bot user.
Once your user has completed the OAuth flow, you'll be able to use the provided tokens to call any of Slack's API methods that require an access token.

See the :ref:`Web API usage <web-api-examples>` section of this documentation for usage examples.
Copy link
Contributor

Choose a reason for hiding this comment

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

This link appears to be broken.

import time

slack_token = os.environ["SLACK_API_TOKEN"]
sc = SlackClient(slack_token)
client = slack.WebClient(slack_token)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should update this and few other places to use keyword-arguments. Replace client = slack.WebClient(slack_token) with client = slack.WebClient(token=slack_token).

******************

Well, poop. Take a deep breath, and then let us know on the `Issue Tracker`_. If you're feeling particularly ambitious,
why not submit a `pull request`_ with a bug fix?
That's great! Thank you. Let us know on the `Issue Tracker`_. If you're feeling particularly ambitious, why not submit a `pull request`_ with a bug fix?
Copy link
Contributor

Choose a reason for hiding this comment

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

These links appear to be broken.

docs-src/faq.rst Outdated

All done? Great! While we're super excited to incorporate your new feature into |product_name|, there are a
couple of things we want to make sure you've given thought to.
All done? Great! While we're super excited to incorporate your new feature into |product_name|, there are a couple of things we want to make sure you've given thought to.
Copy link
Contributor

Choose a reason for hiding this comment

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

I have no idea what product_name is supposed to be. Should we remove this?

@jimray
Copy link
Contributor Author

jimray commented May 30, 2019

  1. Should we keep the v2 docs around for a few weeks so that people who are using those links aren't left with a 404?

I'm of the opinions that we can nuke these from the main repo since they'll still exist in the repo history

  1. I'd like to move to markdown files instead of RestructuredText. Should we do that now or in a subsequent PR?

Definitely agree, let's save that for a subsequent PR. I suspect this will requires some more extensive testing with PyPi and RTD

@shaydewael shaydewael merged commit a22e959 into slackapi:update_docs_v2 Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs M-T: Documentation work only Version: 2x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants