FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Minor typos by samparks · Pull Request #27 · rasbt/python_reference · GitHub

Minor typos - #27

Open
samparks wants to merge 1 commit into
rasbt:masterfrom
samparks:patch-1
Open

Minor typos#27
samparks wants to merge 1 commit into
rasbt:masterfrom
samparks:patch-1

Conversation

Copy link
Copy Markdown

Added a period to *.*format(tn= ...

Added a period to *.*format(tn= ...

rasbt commented Jun 17, 2018

Copy link
Copy Markdown
Owner

Thanks! But I see that there are two periods now:

c.execute("UPDATE {tn} SET {cn}='sebastian_r' WHERE {idf}=123456".\
          .format(tn=table_name, idf=id_column, cn=new_column))

Could you please remove the upper one?

Copy link
Copy Markdown
Author

Ah, sorry! I just realized that the places that I thought periods were needed, were just included above instead of on the new line! I'll let you close this unless you'd like for me to change them all for consistency.

rasbt commented Jun 17, 2018
edited
Loading

Copy link
Copy Markdown
Owner

No worries, and I think it's visually a bit misleading. I think we could leave it as is. It has a bit of those "when you see you old code and cringe" moments ;) I would put the period onto the new line if I wrote it today. Sth like

c.execute("UPDATE {tn} SET {cn}='sebastian_r' WHERE {idf}=123456"
          .format(tn=table_name, idf=id_column, cn=new_column))

(the backslash shouldn't be needed because of the parentheses.)

Copy link
Copy Markdown
Author

👍 sounds good to me. Sorry for the confusion!

rasbt commented Jun 17, 2018

Copy link
Copy Markdown
Owner

No worries, I appreciate it that you submitted a PR helping to fix it :)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL