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

Migrate to new control flow syntax #1827

Merged
merged 2 commits into from Feb 28, 2024
Merged

Conversation

PowerKiKi
Copy link
Contributor

No description provided.

@PowerKiKi PowerKiKi marked this pull request as ready for review February 11, 2024 06:21
@PowerKiKi
Copy link
Contributor Author

Damned! E2e tests are flaky after all 😭 ... any suggestions ?

Out of topic, but you mentioned migrating to signals. I would suggest to release what we have now, without signals. And introduce signals in the next version. Because signals are still a bit young, so probably not everyone is familiar with yet. And because output signals are still missing. I feel it would be weird if we migrate to input signals now, and later migrate to output signals. While it's definitely something we should in mid-long term, I feel we can still postpone a bit. WDTY ?

@santam85
Copy link
Contributor

santam85 commented Feb 13, 2024

Damned! E2e tests are flaky after all 😭 ... any suggestions ?

Out of topic, but you mentioned migrating to signals. I would suggest to release what we have now, without signals. And introduce signals in the next version. Because signals are still a bit young, so probably not everyone is familiar with yet. And because output signals are still missing. I feel it would be weird if we migrate to input signals now, and later migrate to output signals. While it's definitely something we should in mid-long term, I feel we can still postpone a bit. WDTY ?

Yes sure, I just want to have the unit-tests working again first, I think I will revert the jest migration someone else had merged to master.

EDIT: I guess the flakyness could be caused by the linux version used, i generate the screenshots using docker and the node:lts image

@PowerKiKi can you give the docs a once-over and perhaps add a migration guide to v6 🙏 ?

@santam85 santam85 merged commit 1f8491c into valor-software:master Feb 28, 2024
1 check passed
@PowerKiKi
Copy link
Contributor Author

Thank you for the release 🚀

give the docs a once-over

I noticed the content of README.md and apps/ng2-charts-demo/src/app/landing/landing.component.html is almost duplicated, but not quite exactly. What about keeping only one of them ? I'd say let's drop the doc on the site and keep only README.md, which is easier to maintain (no need to escape TS) and offers better syntax highlighting ?

PS: https://valor-software.com/ng2-charts/ was not updated correctly with the last release:

image

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.

None yet

2 participants