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

Drop min schema version required #843

Merged
merged 1 commit into from
Dec 26, 2023
Merged

Conversation

raman325
Copy link
Contributor

We technically don't need to support the latest schema because worst case scenario, we get nothing back for existing rebuild route progress state. If we do the dependency bump to this, it's no longer a breaking change

@MartinHjelmare
Copy link
Contributor

How will we keep track of the compatibility of different server versions and the client version?

@raman325
Copy link
Contributor Author

Are the constants not sufficient? We only need to bump the minimum if a model changes or if there is new key data we are relying on

@MartinHjelmare
Copy link
Contributor

Personally I won't be able to remember what the consequences are of running a server of version 33 with a client that has support for version 34 if we don't have some system for tracking that.

we get nothing back for existing rebuild route progress state

How will this affect the user?

@raman325
Copy link
Contributor Author

raman325 commented Dec 15, 2023

the point of pulling this data in as part of the state dump was so that if a user pulled up the rebuild routes modal after triggering it from elsewhere (e.g. zwave-js-ui), we could immediately display progress. Without it, showing progress is delayed until the next progress event gets emitted.

@raman325
Copy link
Contributor Author

Personally I won't be able to remember what the consequences are of running a server of version 33 with a client that has support for version 34 if we don't have some system for tracking that.

We had a doc that tracked schema changes in the server repo but it's long been abandoned. I can create a task to refresh it up to latest, it will just take a while to parse through all of the changes since the last time it was updated

@MartinHjelmare
Copy link
Contributor

MartinHjelmare commented Dec 17, 2023

History isn't that important. It's the current possible compatibility matrix that is interesting. If a user encounters an issue we're always going to ask them to update to the latest core version. We can skip the previous versions before schema 33 if it's a lot of work.

@raman325
Copy link
Contributor Author

History isn't that imported. It's the current possible compatibility matrix that is interesting. If a user encounters an issue we're always going to ask them to update to the latest core version. We can skip the previous versions before schema 33 if it's a lot of work.

Sorry not following. The server supports all schemas, the issue is the library and the integration. In this case, we can support schema 33 with unimproved functionality but it will still work. So I'm not sure what the action is to take, can you clarify?

@MartinHjelmare
Copy link
Contributor

I was responding to this:

I can create a task to refresh it up to latest, it will just take a while to parse through all of the changes since the last time it was updated

@raman325
Copy link
Contributor Author

gotcha I meant documenting all of the schema changes here: https://github.com/zwave-js/zwave-js-server/blob/master/API_SCHEMA.md

@MartinHjelmare
Copy link
Contributor

Yes. I'm suggesting jumping to schema 33 documenting that and doing the older ones later as time permits. The important information right now is schema 33 and 34.

@raman325
Copy link
Contributor Author

@MartinHjelmare are we good to merge here then?

Copy link
Contributor

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Thanks!

@raman325 raman325 merged commit f97ed18 into home-assistant-libs:master Dec 26, 2023
4 checks passed
@raman325 raman325 deleted the min branch December 26, 2023 03:25
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.

2 participants