Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Minor typos #27

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

Open
samparks wants to merge 1 commit into rasbt:master
base: master
Choose a base branch
Loading
from samparks:patch-1
Open

Minor typos #27

samparks wants to merge 1 commit into rasbt:master from samparks:patch-1

Conversation

Copy link

@samparks samparks commented Jun 17, 2018

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

Added a period to *.*format(tn= ...
Copy link
Owner

rasbt commented Jun 17, 2018

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
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.

Copy link
Owner

rasbt commented Jun 17, 2018
edited
Loading

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
Author

👍 sounds good to me. Sorry for the confusion!

Copy link
Owner

rasbt commented Jun 17, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants

AltStyle によって変換されたページ (->オリジナル) /