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

Update check_doc.py, clang-format-diff.py, zmq_sub.py, optimize-pngs.py print function syntax #15

Merged
merged 5 commits into from
Mar 2, 2023

Conversation

NoNameDude
Copy link

@NoNameDude NoNameDude commented Feb 13, 2023

Updated python print functions

Updated Python code
Updated Python
Updated Python
Updated Python
@farsider350
Copy link

If you could add an edit into the credit section of each file as MarcoFalke has done most the previous work here and either way is sufficient imo.
Not needed but happy to sign off once credits are fixed.

@seanPhill
Copy link

seanPhill commented Feb 14, 2023

Well, I've satisfied myself that the change is necessary, as in Python2 print "something" works, and print ("something") also works, while in Python3 the former fails and the latter succeeds.

Python 2.7.17 (default, Oct  6 2020, 23:05:51) 
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.11.45.5)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> print "something"
something
>>> print ("something")
something
>>> 
Python 3.10.10 (main, Feb  8 2023, 05:44:38) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> print "something"
  File "<stdin>", line 1
    print "something"
    ^^^^^^^^^^^^^^^^^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print(...)?
>>> print ("something")
something
>>> 

@seanPhill
Copy link

seanPhill commented Feb 14, 2023

add an edit into the credit section of each file

In order to do this in a concise and standardised way I'm trying to find an example of adding a minor editor in the credits. It doesn't seem to have been done before, except for in release notes.

check-doc.py seems to be the exception.

Or do you mean in the copyright line? Like adding a 2023 copyright line for "Dingocoin developers"? Isn't it a bit much just for editing the print function to meet requirements for Python3 vs Python2?

Copy link

@seanPhill seanPhill left a comment

Choose a reason for hiding this comment

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

The print functions have been updated to include the required (by Python3) parentheses.
This appears to have become especially important now that default Python has shifted from 2.7 to 3.x. My macOS I see, has recently been updated to run Python3.10 by default, whereas for some years had been running 2.x when invoking python without specifying the version.

@Twinky-kms
Copy link

In future PRs and commit messages, please be a bit more concise. "Fix code" is very vague.

A good PR and commit message here would be, Update python print syntax.

@Twinky-kms
Copy link

Twinky-kms commented Feb 14, 2023

Tested ACK

The check_doc.py, clang-format-diff.py, optimize-pngs.py, and zmq_sub.py. Scripts are still broken on 3.8+, so 2.7 is still required. However, this change does not break the 2.7 version as it supports both syntaxes for the print function so I've gone ahead and approved this PR.

In optimize-pngs.py you missed a few print functions, I've gone ahead and pushed updated syntax for them.

Copy link

@Twinky-kms Twinky-kms left a comment

Choose a reason for hiding this comment

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

Approved.

@Twinky-kms Twinky-kms changed the title Fix code Update check_doc.py, clang-format-diff.py, zmq_sub.py, optimize-pngs.py print function syntax Feb 14, 2023
@Twinky-kms
Copy link

Once @farsider350 recommendations have been added and approval is given by @farsider350 this will be merged.

@farsider350
Copy link

We will merge this, add credits to release notes and in future PR's can be made to develop branch.

@farsider350 farsider350 merged commit 47bc5cd into dingocoin:master Mar 2, 2023
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

4 participants