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

feat: support for showing status updates as soon as they change #110

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ffMathy
Copy link

@ffMathy ffMathy commented Sep 30, 2023

@ffMathy
Copy link
Author

ffMathy commented Sep 30, 2023

It might contain breaking changes though. Not sure if it's a problem or not - can anyone assist with deciding this?

@connectdotz
Copy link
Collaborator

@ffMathy, thanks for the PR.

This repository does need some cleanup. We inherited it a few years back that was mainly a javascript/flow code base. Never did fully convert to typescript, so there are lots of duplicate type def, etc.

Clean-up is always welcome, as long as it doesn't break the logic. If it is just type-name change, then I think we are ok. But if the new/old types are not compatible, then we will need to examine them more closely and decide on a case-by-case basis...

does it compile, and all tests passed? A good test will be to use an updated local repo with vscode-jest when you test the changes. It will validate if the types are compatible after changes.

@ffMathy
Copy link
Author

ffMathy commented Oct 3, 2023

Interesting. Thanks for that insight.

I do unfortunately believe this is a breaking change.

I'm wondering though, are anyone other than ourselves consuming it? Do we need to retain compatibility?

I can work around it by applying the mappings instead in the vscode-jest repo, but at the cost of performance.

@connectdotz
Copy link
Collaborator

What kind of breaking change? We can always cut a major release if needed.

Yes this repo is used by others as well, not just vscode-jest. We need to be mindful.

@connectdotz
Copy link
Collaborator

@ffMathy To clarify, if the breaking change means to get rid of our own custom types and replace them with Jest's official types, which is a superset of the old custom type, then I think we are all right. If there is some info missing from the new type, then we should discuss those.

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