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.

Sign up for GitHub

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

Jump to bottom

fix: the tsconfig spec generated for library contains several issues #2584

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: the tsconfig spec generated for library contains several issues #2584

wants to merge 1 commit into from

Conversation

cpaulve-1A
Copy link
Contributor

Proposed change

Align with application generation and reuse angular tsconfig.spec.json

Related issues

@cpaulve-1A cpaulve-1A requested a review from a team as a code owner December 12, 2024 14:58
@nx-cloud Nx Cloud
Copy link

nx-cloud bot commented Dec 12, 2024 •
edited
Loading

View your CI Pipeline Execution ↗ for commit 52a91bf.

Command Status Duration Result
nx run-many --target=test-int ❌ Failed 12m 8s View ↗
nx affected --target=lint --base=remotes/origin... ❌ Failed 29s View ↗
nx run-many --target=test-e2e ✅ Succeeded 10m 36s View ↗
nx run-many --target=build --projects=eslint-pl... ✅ Succeeded <1s View ↗
nx run-many --target=publish --nx-bail --userco... ✅ Succeeded 36s View ↗
nx run-many --target=documentation ✅ Succeeded 1m 21s View ↗
nx affected --target=test --cacheDirectory=D:\a... ✅ Succeeded 13s View ↗
nx run ama-sdk-schematics:build-swagger ✅ Succeeded 1s View ↗
Additional runs (4) ✅ Succeeded ... View ↗

☁️ Nx Cloud last updated this comment at 2025-01-10 18:32:47 UTC

@codecov Codecov
Copy link

codecov bot commented Dec 12, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.26%. Comparing base (a083fe5) to head (52a91bf).
Report is 2 commits behind head on main.

✅ All tests successful. No failed tests found.

Additional details and impacted files

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

fpaul-1A
fpaul-1A reviewed Dec 12, 2024
View reviewed changes
packages/@o3r/workspace/schematics/library/templates/ng/tsconfig.spec.json.template
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an it-test https://github.com/AmadeusITGroup/otter/blob/main/packages/%40o3r/workspace/schematics/index.it.spec.ts#L117 to make sure the test are working on the generated lib?

fpaul-1A
fpaul-1A previously approved these changes Dec 13, 2024
View reviewed changes
@cpaulve-1A cpaulve-1A force-pushed the hotfix/2481 branch 2 times, most recently from 392858a to e700452 Compare December 17, 2024 11:42
@cpaulve-1A cpaulve-1A force-pushed the hotfix/2481 branch 2 times, most recently from 90d7ac8 to 3341ac0 Compare January 6, 2025 14:55
vscaiceanu-1a
vscaiceanu-1a previously approved these changes Jan 6, 2025
View reviewed changes
fpaul-1A
fpaul-1A previously approved these changes Jan 7, 2025
View reviewed changes
@cpaulve-1A cpaulve-1A dismissed stale reviews from fpaul-1A and vscaiceanu-1a via f02ac1a January 7, 2025 17:09
kpanot
kpanot reviewed Jan 8, 2025
View reviewed changes
packages/@o3r/workspace/schematics/library/templates/ng/tsconfig.spec.json.template Outdated
@@ -1,17 +0,0 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this file is removed?
Is the default one align with this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not have it for the applications and the values were incorrect. Composite did not work and ng test could not resolve the paths to the source files.

Copy link
Contributor

Choose a reason for hiding this comment

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

composite does not work with jasmine indeed but does work with jest.
We do recommend to have it for the application (for type and config separating purpose mainly).
To be able to resolve the paths it should extends the tsconfig.base.json from the root, if we remove this file it may be more complex to fix it :S

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kpanot kpanot kpanot left review comments

@fpaul-1A fpaul-1A fpaul-1A left review comments

@vscaiceanu-1a vscaiceanu-1a vscaiceanu-1a left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
bug Something isn't working project:new-version-gh-action project:@o3r/test-helpers project:@o3r/workspace
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: The tsconfig spec generated for library contains several issues
4 participants
@cpaulve-1A @kpanot @vscaiceanu-1a @fpaul-1A