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 3 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 3 commits 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

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 628f7e6. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

🟥 Failed Commands
nx affected --target=lint --base=remotes/origin/main --configuration ci
✅ Successfully ran 10 targets

Sent with 💌 from NxCloud.

@codecov Codecov
Copy link

codecov bot commented Dec 12, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.

Project coverage is 70.88%. Comparing base (5cb8aeb) to head (628f7e6).
Report is 43 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
.../@o3r/workspace/schematics/library/rules/shared.ts 69.23% 3 Missing and 1 partial ⚠️
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 Outdated
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 commented Dec 18, 2024
View reviewed changes
packages/@o3r/workspace/schematics/ng-add/helpers/npm-workspace.ts
@@ -115,9 +115,10 @@ export function addMonorepoManager(o3rWorkspacePackageJson: PackageJson & { gene
lint: 'lerna run lint'
};

const lernaJson: { $schema: string; version: string; npmClient?: string } = {
const lernaJson: { $schema: string; version: string; npmClient?: string; useNx?: boolean } = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove this change, wrongly commited

kpanot
kpanot reviewed Dec 19, 2024
View reviewed changes
packages/@o3r/test-helpers/src/utilities/package-manager.ts
@@ -297,7 +297,7 @@ export function setPackagerManagerConfig(options: PackageManagerConfig, execAppO
execFileSync('yarn', ['config', 'set', 'enableGlobalCache', 'true'], execOptions);
execFileSync('yarn', ['config', 'set', 'globalFolder', options.globalFolderPath], execOptions);
}
execFileSync('yarn', ['config', 'set', 'nodeLinker', 'pnp'], execOptions);
execFileSync('yarn', ['config', 'set', 'nodeLinker', 'node-modules'], execOptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

The purpose of the e2e with yarn is to test the default config of yarn which is PNP.
Not sure we should change it :S.
Why did you need to?

packages/@o3r/workspace/schematics/library/index.spec.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

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

@kpanot kpanot kpanot 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
3 participants
@cpaulve-1A @kpanot @fpaul-1A