Skip to content

Upgrade dependencies and implementation of the integration test #827

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

Closed
wants to merge 4 commits into from

Conversation

stockiNail
Copy link
Collaborator

This PR is changing the dependency with CHART.JS, going to version4. It's also adding the test on TS 4.7.x

@stockiNail stockiNail added this to the 2.2.0 milestone Jan 4, 2023
Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Maybe it should ve testing both?

@stockiNail
Copy link
Collaborator Author

Maybe it should ve testing both?

Chart.js 3 and 4?

@kurkle
Copy link
Member

kurkle commented Jan 4, 2023

Yes, or was v3 support already droped?

@stockiNail
Copy link
Collaborator Author

V3 should be supported, version 3.7.0

@stockiNail
Copy link
Collaborator Author

I will work tomorrow on that. I think we need a package.json template to update at runtime, settimg v3 and then v4. I need to see how the fs api in js are working

@kurkle
Copy link
Member

kurkle commented Jan 5, 2023

Its already running for each of the ts versions, should not be hard to add another loop for that. Or just split to separate folders (and maybe test only some of the later ts versions with v4)

@stockiNail stockiNail marked this pull request as draft January 5, 2023 07:47
@stockiNail
Copy link
Collaborator Author

stockiNail commented Jan 5, 2023

Its already running for each of the ts versions, should not be hard to add another loop for that. Or just split to separate folders (and maybe test only some of the later ts versions with v4)

@kurkle I have done! Have a look if ok.
Let me say that the implementation is getting the devDep and peerDep of the plugin to chart.js and based on that it calculates the amount of majors between them. And then, configure the package.json for testing to use the last version in the major, by [major].x.x.
In this way we don't have to change anything next time, when the plugin will change the dependencies to Chart.js.

[test-*types-integration] added 9 packages, and audited 10 packages in 15s
[test-*types-integration] 
[test-*types-integration] found 0 vulnerabilities
[test-*types-integration] 
[test-*types-integration] > test
[test-*types-integration] > node test.js
[test-*types-integration] 
[test-*types-integration] Testing on typescript-4.7 ...
[test-*types-integration] Testing on typescript-4.6 ...
[test-*types-integration] Testing on typescript-4.5 ...
[test-*types-integration] Testing on typescript-4.4 ...
[test-*types-integration] Testing on typescript-4.3 ...
[test-*types-integration] Testing on typescript-4.2 ...
[test-*types-integration] Testing on typescript-4.1 ...
[test-*types-integration]     ✔ chartjs-plugin-annotation should compile with all supported TS versions on Chart.js version 3.x.x (51533ms)
[test-*types-integration] 
[test-*types-integration] added 10 packages, and audited 11 packages in 4s
[test-*types-integration] 
[test-*types-integration] found 0 vulnerabilities
[test-*types-integration] 
[test-*types-integration] > test
[test-*types-integration] > node test.js
[test-*types-integration] 
[test-*types-integration] Testing on typescript-4.7 ...
[test-*types-integration] Testing on typescript-4.6 ...
[test-*types-integration] Testing on typescript-4.5 ...
[test-*types-integration] Testing on typescript-4.4 ...
[test-*types-integration] Testing on typescript-4.3 ...
[test-*types-integration] Testing on typescript-4.2 ...
[test-*types-integration] Testing on typescript-4.1 ...
[test-*types-integration]     ✔ chartjs-plugin-annotation should compile with all supported TS versions on Chart.js version 4.x.x (23987ms)

@stockiNail stockiNail marked this pull request as ready for review January 5, 2023 10:58
@stockiNail stockiNail requested a review from kurkle January 5, 2023 10:58
@stockiNail stockiNail changed the title Upgrade dependencies in the integration test Upgrade dependencies and implementation of the integration test Jan 5, 2023
@stockiNail stockiNail marked this pull request as draft January 26, 2023 11:27
@stockiNail
Copy link
Collaborator Author

Must be rebased when #838 will be merged. Now in draft

@kurkle
Copy link
Member

kurkle commented Jan 26, 2023

Many lines of code to avoid adding a chart.js major once a year or two? :)
Less maintenance is less maintenance, so why not.

@kurkle
Copy link
Member

kurkle commented Jan 26, 2023

Another thought, maybe this one should be converted to pnpm too, so it could use workspace dependencies and avoid the temporary directory stuff. Like its done in chart.js now.

@stockiNail
Copy link
Collaborator Author

Many lines of code to avoid adding a chart.js major once a year or two? :)
Less maintenance is less maintenance, so why not.

I could agree. For my mindset, I'm never afraid about maintenance or LoC ;)

Another thought, maybe this one should be converted to pnpm too, so it could use workspace dependencies and avoid the temporary directory stuff. Like its done in chart.js now.

OK! for this one, I need more time to have a look how it works with more details that I have got now.

@stockiNail stockiNail closed this Jan 27, 2023
@stockiNail
Copy link
Collaborator Author

Close for now

@stockiNail stockiNail removed this from the 2.2.0 milestone Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants