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

Invoke format_comment even in dry run. Fixes #20. #35

Merged
merged 1 commit into from
Dec 19, 2015
Merged

Invoke format_comment even in dry run. Fixes #20. #35

merged 1 commit into from
Dec 19, 2015

Conversation

jaraco
Copy link
Collaborator

@jaraco jaraco commented Dec 19, 2015

I haven't tested this change, but the idea is basically to invoke the comment formatting during the dry run.

jeffwidman added a commit that referenced this pull request Dec 19, 2015
Invoke format_comment even in dry run. Fixes #20.
@jeffwidman jeffwidman merged commit 6bbd1a4 into jeffwidman:master Dec 19, 2015
@jaraco jaraco deleted the format-comment branch December 19, 2015 17:10
@jeffwidman
Copy link
Owner

Quick FYI for the archives--while the idea here is sound, it's currently throwing an exception for me whenever I try to use the --dry-run option on master at 27c5004

I thought I'd manually tested it properly before merging, but obviously I screwed up...
I was simultaneously working on some other changes to the code and I think I accidentally tested the wrong branch.

I'm just about to go take a look at #36, which I'm guessing will fix this due to the huge refactor, but did want to add a note here just for reference.

The exception I'm hitting:

File "migrate.py", line 363, in <module>
    print u"Comments", [format_comment(options, comment['body'].encode('utf-8', errors='replace')) for comment in comments]
  File "migrate.py", line 131, in format_comment
    comment['user'],
TypeError: string indices must be integers, not str

@jeffwidman jeffwidman mentioned this pull request Dec 20, 2015
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