Skip to content

Conversation

@jlaneve
Copy link
Contributor

@jlaneve jlaneve commented May 14, 2021

Fixes formatting on SlackHook's docstring to adhere to Sphinx standards. See the Airflow docs for the current rendering - the short description is currently:

"Creates a Slack connection, to be used for calls. Takes both Slack API token directly and connection that has Slack API token. If both supplied, Slack API token will be used. Exposes also the rest of slack.WebClient args Examples:"

Also fixes minor grammatical issues.

@github-actions
Copy link

The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest master or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label May 14, 2021
@potiuk
Copy link
Member

potiuk commented May 14, 2021

You need reformat the files - staic checks are failing.

@drdenzy
Copy link
Contributor

drdenzy commented May 14, 2021

If you are using VS Code, you can use black to reformat the file and that should fix the static check error.

@potiuk
Copy link
Member

potiuk commented May 14, 2021

Or even better. Run pre-commit install and it will be automatically fixed for you when you run git commit

@jlaneve
Copy link
Contributor Author

jlaneve commented May 14, 2021

@potiuk I think they're fixed now but there's one task still failing. I can't tell what the error is / if it's caused by my fix. It doesn't make sense that it would be given it's a Postgres failure but can you confirm?

@kaxil kaxil merged commit 6d9fc3e into apache:master May 14, 2021
@kaxil
Copy link
Member

kaxil commented May 14, 2021

Thanks @jlaneve for the PR

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

Labels

area:providers okay to merge It's ok to merge this PR as it does not require more tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants