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

fix PrepareSequenceAcqusition typo #453

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tlambert03
Copy link
Contributor

@marktsuchida ... I'm not really sure about the repercussions of fixing a typo like this. Is this something that one just has to live with forever now? would it require a change in the device interface version?

@tlambert03
Copy link
Contributor Author

would it require a change in the device interface version?

i think it probably does. so I bumped it here. But if that is too invasive of a procedure just to fix a typo, feel free to close this

@nicost
Copy link
Member

nicost commented Mar 26, 2024

Yes, this will need a bump in device interface version (easy to see how this could crash when mixing binaries from different versions). The problem with device interface version changes is that it is disruptive for device adapters that are not living in the Micro-Manager repository. For instance, I have binary only adapters from Gataca and Mightex and it will be pulling teeth to get updates versions, even though the only need to recompile. Wondering if it is possible to go into binaries and change that version number manually, should be harmless in a case like this - where no functions present in those adapters are changed.

I am in favor of waiting to merge this until the device interface version needs to be changed for other reasons as well. Hopefully we will remember this PR by that time...

@tlambert03
Copy link
Contributor Author

Ok! No urgency at all obviously :)

@marktsuchida
Copy link
Member

Sorry for the late reply. I've known about this misspelling for a decade and avoided fixing it ("if it ain't broke"), but it'll be good to clean up.

I'm 75% sure that we don't need to bump the device interface version for this change (the function is only accessed via a pointer in the vtable, and I think its slot doesn't change by renaming), and would rather avoid bumping it if possible.

Let me see if I can prove that a bump is not needed -- otherwise let's keep this around until the next time we need to bump the device interface version anyway and do it then. I was meaning to make a "things to fix when we bump the DIV" super-issue, and there is one particular breaking change that I want to propose soon.

@tlambert03 tlambert03 mentioned this pull request Oct 9, 2024
@marktsuchida
Copy link
Member

We should be able to merge this relatively soon, since we'll be bumping the device interface version for other reasons.

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.

3 participants