New Architect API #13463
Merged
New Architect API #13463
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hansl
force-pushed
the
architect-poc
branch
4 times, most recently
from
6d238c0 to
991daff
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
hansl
force-pushed
the
architect-poc
branch
5 times, most recently
from
2028785 to
ca13cac
Compare
This comment has been minimized.
This comment has been minimized.
hansl
force-pushed
the
architect-poc
branch
2 times, most recently
from
e39bf1c to
933628a
Compare
hansl
force-pushed
the
architect-poc
branch
4 times, most recently
from
f3e9a20 to
cb2cd52
Compare
clydin
reviewed
Feb 15, 2019
kyliau
approved these changes
Feb 19, 2019
|
|
||
| return { | ||
| ...targetSpec['options'], | ||
| ...(target.configuration ? targetSpec['configurations'][target.configuration] : 0), |
There was a problem hiding this comment.
just curious, is calling spread operator on a number actually valid syntax?
I tried it in TS playground and it complains about "Spread types may only be created from object types."
https://www.typescriptlang.org/play/#src=console.log(%7B%20...(1%20%2B%201%20%3D%3D%3D%202%20%3F%203%20%3A%204)%20%7D)%3B
There was a problem hiding this comment.
As per offline discussion, { ...0 } is valid javascript.
| info: info, | ||
| options: buildOptions, | ||
| ...(options.target ? { target: options.target } : {}), | ||
| }); |
There was a problem hiding this comment.
nit: refactor this object so it doesn't have to be repeated below.
clydin
reviewed
Feb 19, 2019
| @@ -6,4 +6,5 @@ | |||
| * found in the LICENSE file at https://angular.io/license | |||
| */ | |||
| export * from './text'; | |||
| export * from './caps'; | |||
There was a problem hiding this comment.
This doesn't appear to be used anywhere?
There was a problem hiding this comment.
It is in the architect CLI.
There was a problem hiding this comment.
(see the progress bar stuff).
clydin
approved these changes
Feb 19, 2019
It was only used when manually testing the old Architect.
It was possible to overwrite the metadata of the logger itself (name, path) when calling "log()". This should not happen. If there is a need to overwrite the loggers metadata itself one should use "next()" and construct or forward their own log entry.
Current behaviour is to have logs forwarded, but this is flawed because on the job side the logger is actually re-created. This allows logs to be actually part of the caller side logging infrastructure.
The new API has been described in this design doc: https://docs.google.com/document/d/1SpN_2XEooI9_CPjqspAcNEBjVY874OWPZqOenjuF0qo/view This first drafts add support for the API (given some deep imports). It is still in draft mode but is committed to make it available to people to start testing and moving their own builders. This API rebuilds (not backward compatible) the Architect API package. To use it people will need to import "@angular-devkit/architect/src/index2" to start using it. A reference builder will be added in the next commit. There are 2 pieces missing from this commit that will be added in the same PR; 1) the architect-host and CLI to test, and 2) a reference builder moved from the old API to the new one. These will be part of the same PR. Finally, there are missing tests in this package, but everything tested manually and automatically works so far. Test coverage will be added before the package is considered finished. Due to a desire to keep architect, our tests and the scope of this PR limited and keep the two APIs separated, every clashing files will have a "2" suffix added to it. Once all builders have been moved and we are sure everything works, all those files will be moved to their final destination and the old API will be removed, in one PR.
This host resolves using the package resolution and reading the targets from the workspace API.
Four builders were added; - true, always succeed - false, always fails - concat, runs all targets or builders in succession - allOf, runs all targets or builders in parallel
It is only new files and the old builder is still available. The new one can only be used by the new Architect API.
Move the entire Architect CLI to use the new API, and report progress using a progress bar for each worker currently executing. Shows log at the end of the execution. This is meant to be used as a debugging tool to help people move their builders to the new API.
Also tighten the types a bit, and add a test that failed before and works now.
If a system wants to have logging it should multiplex it itself on a channel. Also changed the previous Architect commits to remove usage of Logs and move to a "log" channel.
|
Just force pushed the |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For now just a proof of concept, but this branch will be reused for the final implementation.