Skip to content
This repository was archived by the owner on Jan 26, 2024. It is now read-only.

Conversation

@JohannesHoppe
Copy link
Member

Hello, gentlemen,

I'm pretty sure that the signature of the ngAdd schematic was broken. The affected code was directly copied from ngx-gh, so later on we might want to double-check that nobody else copied that error, too. In the end the value of options.project was always undefined. This was reported in #5.

BEFORE:

export const ngAdd = ({ project: DeployOptions }) => (
  tree: Tree,
  options: NgAddOptions
)

AFTER

export const ngAdd = (options: NgAddOptions) => (
  tree: Tree,
  _context: SchematicContext
)

I also didn't understand the original test:
https://github.com/mgechev/ngx-gh/blob/5088488a50af7550eb6b5b0dfcfac4ade6a61085/ng-add.spec.ts#L36-L47

Why should one installation override the next one in case of multiple projects? In the current implementation, both simply changes remain.
@mgechev could you please review this and tell me if I'm on the right track?

FYI @dianjuar

Copy link

@mgechev mgechev left a comment

Choose a reason for hiding this comment

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

LGTM

@JohannesHoppe
Copy link
Member Author

👍

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants