Skip to content

Comments

feat(schematics): ng-add schematics for adding element dependencies and styling configuration#1536

Open
mistrykaran91 wants to merge 1 commit intomainfrom
feat/ng-add-schematics
Open

feat(schematics): ng-add schematics for adding element dependencies and styling configuration#1536
mistrykaran91 wants to merge 1 commit intomainfrom
feat/ng-add-schematics

Conversation

@mistrykaran91
Copy link
Member

Describe in detail what your merge request does and why. Add relevant
screenshots and reference related issues via Closes #XY or Related to #XY.


Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces ng-add schematics for @siemens/element-ng, which is a great enhancement for user experience. The implementation is well-structured, separating dependency management and styling configuration into different files and schematics. The use of tasks to run installation before style configuration is correct. I've found one area for improvement regarding code duplication and performance in add-dependencies.ts, which I've detailed in a specific comment. Overall, this is a solid implementation.

@github-actions
Copy link

@mistrykaran91 mistrykaran91 force-pushed the feat/ng-add-schematics branch 3 times, most recently from 594d04a to 8a44a03 Compare February 20, 2026 07:15
@github-actions
Copy link

Code Coverage

@mistrykaran91 mistrykaran91 marked this pull request as ready for review February 20, 2026 09:52
@mistrykaran91 mistrykaran91 requested review from a team as code owners February 20, 2026 09:52
addDependency('@siemens/element-theme', elementVersion, options),
addDependency('@siemens/element-translate-ng', elementVersion, options),
addDependency('@angular/cdk', cdkVersion, options),
addDependency('@simpl/brand', '2.2.0', options),
Copy link
Member

Choose a reason for hiding this comment

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

I guess this version is already outdated. Can you please add a unit test, that checks that this version is inline with the one from root package.json

*/
export const addElementDependencies = (): Rule => {
return (tree: Tree, context: SchematicContext) => {
const packageJsonPath = join(dirname(fileURLToPath(import.meta.url)), '../../package.json');
Copy link
Member

Choose a reason for hiding this comment

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

what about just having a normal import statement for our package.json?

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