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

Remove comtypes requirement #1444

Merged
merged 1 commit into from
Nov 2, 2020
Merged

Conversation

wkschwartz
Copy link
Contributor

Fixes #1443

What's missing:

  • A news item for docs/whatsnew.rst because I'm not sure how you handle in-development items.
  • I didn't touch the docs/locales/**/*.po files that popped up with git grep --files-with-matches comtypes because they look old and I don't know how to use them anyway.
  • I haven't added or changed any tests because what's to test?
  • I haven't run the tests because I'm on my mac today and I'm hoping your CI will just do it.

@fzumstein
Copy link
Member

This looks awesome! Yes, definitely a good move also in light of this. There are no unit tests for this functionality. Instead, the following has to be tested manually (which I did and I couldn't find any issues):

  • Call RunPython with an Excel workbook from an untrusted location, e.g. downloaded from the Internet
  • Open the same workbook in two instances of Excel and make sure RunPython still works on the correct workbook

@wkschwartz
Copy link
Contributor Author

Great. What remains to be done before merger? Anything for docs/whatsnew.rst or docs/locales/**/*.po?

@fzumstein
Copy link
Member

Nothing to be done, I'll take care of that before release.

@fzumstein fzumstein merged commit c2807b9 into xlwings:master Nov 2, 2020
@wkschwartz wkschwartz deleted the rm-comtypes branch April 29, 2022 05:14
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.

Drop comtypes requirement
2 participants