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

adjust the judgment logic of calling UndoRecord interfacex #68

Open
wants to merge 1 commit into
base: 6.0
Choose a base branch
from

Conversation

ridewindgo
Copy link

In the past, whether to call the UndoRecord interface based on the word version. Now adjust to: checking whether the UndoRecord interface exists first, then decide to whether call the UndoRecord interface, this seems more safe and easier to understand.

…rd version. Now adjust to: checking whether the UndoRecord interface exists first, then decide to whether call the UndoRecord interface.
@adomasven
Copy link
Member

So generally we recommend posting to the zotero-dev mailing list if you want to make specific contributions to Zotero code. A good place to start is when coming up with fixes is with issues available on Zotero repos.

As far as UndoRecord is concerned, since this code has been added, we have actually dropped support on Windows for Word versions earlier than 2010, so we could just drop the conditional.

On the other hand, we generally don't fix what's not broken, including legacy handling code, because while we may not officially provide support and bugfixes for older versions of software, if users are able to get that to run and work on their machines, we're happy for them to do so. I assume you tested whether this works, but I'm not sure why this change is needed, aside from the fact that checking for functionality is generally the preferential way to build integrations as opposed to checking for version. Is there any specific reason why you think this change is necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants