Skip to content

feat(@angular-devkit/build-ng-packagr): implement stable architect API #13877

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

Merged
merged 1 commit into from
Mar 19, 2019

Conversation

clydin
Copy link
Member

@clydin clydin commented Mar 11, 2019

This is a continuation of the Angular builder refactoring following the introduction of the new stable architect builder API.

@clydin clydin added the target: major This PR is targeted for the next major release label Mar 11, 2019
@clydin clydin force-pushed the builder-lib-update branch 2 times, most recently from c6559d5 to 0e3a225 Compare March 11, 2019 17:45
@clydin clydin requested a review from alan-agius4 March 11, 2019 19:03
@hansl hansl self-requested a review March 11, 2019 19:46
Copy link
Contributor

@hansl hansl left a comment

Choose a reason for hiding this comment

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

One nit from me.

@@ -1,4 +1,5 @@
{
"$schema": "http://json-schema.org/draft-07/schema",
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering the draft won't be changing much, we should start using a non-draft version of the schema spec; http://json-schema.org/schema.

Copy link
Member Author

Choose a reason for hiding this comment

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

http://json-schema.org/schema points to latest version of the schema and will change over time. It currently points to the draft-07 version which allows everything to work for now. However, there is no guarantee that the latest version will be supported by the CLI nor that the file will conform to the latest version. By specifying a specific version the $schema identifier can be used as both a version identifier for the format of the file and as the location of the schema. This allows processors to use the specified version/format of the specification when parsing and processing the file. (ref: http://json-schema.org/draft-07/json-schema-core.html#rfc.section.7)

Copy link
Contributor

Choose a reason for hiding this comment

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

(we use that value too when running quicktype)

This is non blocking, but we use the "latest" URI in all but 2 of our schemas, so I'd say it's better to use the same everywhere than just keep splitting the code base.

@clydin clydin force-pushed the builder-lib-update branch 2 times, most recently from 0e1c132 to 490467c Compare March 13, 2019 23:56
Copy link
Collaborator

@alan-agius4 alan-agius4 left a comment

Choose a reason for hiding this comment

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

LGTM

@clydin clydin force-pushed the builder-lib-update branch from 490467c to a9fc9fa Compare March 14, 2019 14:59
@clydin clydin force-pushed the builder-lib-update branch from a9fc9fa to 7f0e037 Compare March 16, 2019 00:25
@alexeagle alexeagle merged commit fc48101 into angular:master Mar 19, 2019
@clydin clydin deleted the builder-lib-update branch March 19, 2019 16:21
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants