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

Tidy messages for python2to3 #62

Merged
merged 1 commit into from
Jul 10, 2017
Merged

Tidy messages for python2to3 #62

merged 1 commit into from
Jul 10, 2017

Conversation

adamchainz
Copy link
Owner

@adamchainz adamchainz commented Jun 23, 2017

My logic here: anything mentioning a six replacement doesn't need mention of Python 2/3 because that's what six is. For everything else, mention if it was removed or moved, and point to the solution.

@adamchainz
Copy link
Owner Author

WIP

'cStringIO.cStringIO': 'moved in Python 3. io.BytesIO can be used as a drop-in replacement',
'cStringIO.StringIO': 'moved in Python 3. six.moves.cStringIO can be used as a drop-in replacement',
'Dialog': 'moved in Python 3. six.moves.tkinter_dialog can be used as a drop-in replacement',
'commands': 'removed in Python 3, use subprocess',
Copy link
Contributor

Choose a reason for hiding this comment

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

I like most of the shorter messages, since "moved in python 3" doesn't add extra extra information, but I'm a fairly disappointed to see the "drop-in replacement" language removed.

Some libraries are not precise substitutes. "Drop-in replacement" means that I devoted effort to checking they're the same. For example, this PR makes commands (for which subprocess is not a drop-in replacement) have the same message as ConfigParser (for which six.moves.configparser is a drop-in replacement). Some of the other cases may in fact be exact substitutes or I may have made errors, but I've found this to be useful information.

If I'd known you were going to remove those, I would not have bothered to submit the PR originally, since this will make the released version much less useful than my previous bulky configuration file.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sorry, this is still WIP, I haven't made my mind up. Thanks for pointing this out, maybe we should be calling out the NON drop in replacements instead? Also I've been thinking about adding detection for non-from-imports too.

My logic here: anything mentioning a `six` replacement doesn't need mention of Python 2/3 because that's what six is. For everything else, mention if it was removed or moved, and point to the solution.
@adamchainz
Copy link
Owner Author

@lucaswiman finally got around to finishing this up. I kept the "as a drop-in replacement" language since I think it's still the clearest way to explain this, so this change isn't doing so much. I've made #63 for future detection of using of banned imports later in the code, it's a lot harder since it involves name resolution in the code.

@adamchainz adamchainz merged commit 33888b2 into master Jul 10, 2017
@adamchainz adamchainz deleted the python2to3_tidy branch July 10, 2017 10:36
@lucaswiman
Copy link
Contributor

This is great, @adamchainz! Thanks so much!

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

2 participants