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

Consistency: apply codestyle formatting to docstring code snippets #1220

Closed
wants to merge 3 commits into from
Closed

Consistency: apply codestyle formatting to docstring code snippets #1220

wants to merge 3 commits into from

Conversation

jayaddison
Copy link
Contributor

Description

This is a pedantic/consistency follow-up from #1219: that change applied some black formatting to two code snippets, and this change applies that formatting to the remaining snippets in the codebase.

For each snippet, I extracted the code and applied formatting using black v23.1.0, then placed the results back into the source.

In one case there was an associated 'output' block, and in that case I re-ran the snippet code to update the results of that output too (this included a change-in-output thanks to a bugfix since the snippet was written, by the looks of it).

Checklist

This pull request is:

  • A documentation / typographical error fix

…avoid adjusting the standard use of double-quotes in triple-quoted docstrings)
…ception (to avoid adjusting the standard use of double-quotes in triple-quoted docstrings)
@CaselIT
Copy link
Member

CaselIT commented Apr 13, 2023

thanks for the effort!

Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision cd65a45 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

New Gerrit review created for change cd65a45: https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4566

@sqla-tester
Copy link
Collaborator

Federico Caselli (CaselIT) wrote:

only formatting of code inside the docs.

should be good to go once it passes the pipeline

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4566

@CaselIT
Copy link
Member

CaselIT commented Apr 13, 2023

In one case there was an associated 'output' block, and in that case I re-ran the snippet code to update the results of that output too (this included a change-in-output thanks to a bugfix since the snippet was written, by the looks of it).

judging by the u there it seems the last time it was run was on python 2!

Thanks for updating it

@jayaddison
Copy link
Contributor Author

You're welcome, and thanks - funny the things we notice and the things we don't :)

@jayaddison
Copy link
Contributor Author

(if that was ambiguous: I didn't mean that there is anything else hidden in the changes - I was only surprised that I noticed the missing object reference and yet didn't notice the u prefix at all.. probably a result of seeing so many changesets where that exact difference occurs, often repetitively)

@sqla-tester
Copy link
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/4566 has been merged. Congratulations! :)

@jayaddison jayaddison deleted the docstrings/snippet-format-consistency branch April 25, 2023 19:06
@jayaddison
Copy link
Contributor Author

Thank you, @CaselIT!

@CaselIT
Copy link
Member

CaselIT commented Apr 25, 2023

Thanks you for doing most of the work!

@jayaddison
Copy link
Contributor Author

Thanks you for doing most of the work!

I'm fairly sure that you didn't intend this meaning, but the word 'most' made me double-check.. and I found that I missed a snippet within Operations.create_table.

(and, a smaller detail than that: one of the create_primary_key snippets could be simplified onto a single-line)

Is that worth a follow-up pull request?

@CaselIT
Copy link
Member

CaselIT commented Apr 26, 2023

I'm fairly sure that you didn't intend this meaning

sorry if I gave you that impression, with most I just meant that I though I added a changelog but apparently not, so I didn't do much here at all here :)

Is that worth a follow-up pull request?

Sure, if you have time it would be appreciated

@jayaddison
Copy link
Contributor Author

I'm fairly sure that you didn't intend this meaning

sorry if I gave you that impression, with most I just meant that I though I added a changelog but apparently not, so I didn't do much here at all here :)

It's OK - I didn't read it that way, although am glad that it was worth checking. It would have been better for me to have noticed it myself before the first PR, of course!

Is that worth a follow-up pull request?

Sure, if you have time it would be appreciated

Great - yep, I'll provide a pull request in a few moments.

sqlalchemy-bot pushed a commit that referenced this pull request Apr 28, 2023
… docstrings

### Description
Follow-up / completion of #1220.  That change updated a number of docstrings within the codebase to use standardised `black` code formatting, but a couple of locations had been missed.

### Checklist

This pull request is:

- [x] A documentation / typographical error fix
	- Good to go, no issue or tests are needed

Closes: #1228
Pull-request: #1228
Pull-request-sha: f5696b9

Change-Id: I5d935b036d6f4e11eb5c229f9982db587d67ae24
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.

None yet

3 participants