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

Add missing commas to the contrib.sla module code examples. #1972

Merged
merged 1 commit into from
Dec 26, 2016

Conversation

aliavni
Copy link
Contributor

@aliavni aliavni commented Dec 23, 2016

Description

Code examples in the luigi.contrib.sqla module were misleading with the missing commas in tuples.

Motivation and Context

This code example is copied and pasted in other resources than the Luigi documentation (e.g., http://www.tuicool.com/articles/bumiYr). The motivation here is a better documentation.

Have you tested this? If so, how?

I built the documentation with tox -e docs and confirmed the changes.

@mention-bot
Copy link

@aliavni, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Tarrasch, @soxofaan and @graingert to be potential reviewers.

Copy link
Contributor

@graingert graingert left a comment

Choose a reason for hiding this comment

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

Any way of testing this?

@aliavni
Copy link
Contributor Author

aliavni commented Dec 23, 2016

@graingert do you mean other than building the docs with tox -e docs and confirming the changes?

@graingert
Copy link
Contributor

doctest? Run a linter over the code blocks?

@Tarrasch Tarrasch merged commit 9e29e6d into spotify:master Dec 26, 2016
@Tarrasch
Copy link
Contributor

Tarrasch commented Dec 26, 2016

@graingert , do you want to take a short look at https://pypi.python.org/pypi/flake8-docstrings? It seems all you need to do is to add a line to the tox file. I saw this example but I think it's outdated.

kreczko pushed a commit to kreczko/luigi that referenced this pull request Mar 28, 2017
This was referenced Jun 29, 2022
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