Repository navigation
fix(docs-infra): convert hard-coded cli-builder examples into proper mini-app - #34362
sonukapoor wants to merge 1 commit into
Conversation
gkalpak
added
aio: preview
area: build & ci
|
You can preview 5ff281b at https://pr34362-5ff281b.ngbuilds.io/. |
There was a problem hiding this comment.
Nice! I left a few comments, but this is definitely on the right track 👍
Thx, @sonukapoor ✨
BTW, why the empty main.ts file?
|
You can preview c776fb0 at https://pr34362-c776fb0.ngbuilds.io/. |
gkalpak
left a comment
gkalpak
left a comment
There was a problem hiding this comment.
Some more (mostly minor) comments to go 👏
@sonukapoor, could you also create a new GitHub issue to track adding tests for the cli-builder example?
|
|
You can preview f752948 at https://pr34362-f752948.ngbuilds.io/. |
|
You can preview 07ab466 at https://pr34362-07ab466.ngbuilds.io/. |
gkalpak
left a comment
gkalpak
left a comment
There was a problem hiding this comment.
One last comment to go 🎉
Can you also squash all commits together into one and change the commit message to something like:
fix(docs-infra): convert hard-coded `cli-builder` examples into a proper mini-app
Previously, the examples in the `cli-builder` guide were hard-coded.
This made it impossible to test them and verify they are correct.This commit fixes this by converting them into a proper mini-app. In a
subsequent commit, tests will be added to verify that the source code
works as expected (and guard against regressions).Fixes #34314
BTW, Closes #34314 would work too, but not Closes: #34314 (there should be no semi-colon).
…per mini-app Previously, the examples in the `cli-builder` guide were hard-coded. This made it impossible to test them and verify they are correct. This commit fixes this by converting them into a proper mini-app. In a subsequent commit, tests will be added to verify that the source code works as expected (and guard against regressions). Fixes angular#34314
07ab466 to
dd9793f
Compare
|
You can preview dd9793f at https://pr34362-dd9793f.ngbuilds.io/. |
gkalpak
left a comment
gkalpak
left a comment
There was a problem hiding this comment.
Great job, @sonukapoor 👏
(Thx for seeing this through ⭐️)
|
@mgechev this is one that you probably want to take a look at. |
gkalpak
added
action: merge
kara
added
the
action: review
gkalpak
changed the title
cli-builder examples into proper mini-app
|
@IgorMinar Could you please review this one? Thanks |
Splaktar
left a comment
Splaktar
left a comment
There was a problem hiding this comment.
LGTM. This fixes a number of issues that can cause confusion when going through this guide. It would be helpful to get this into version 9.
IgorMinar
added
action: merge
gkalpak
added
the
merge: caretaker note
|
@caretaker: Global approvals for the docs-only changes. |
…per mini-app (#34362) Previously, the examples in the `cli-builder` guide were hard-coded. This made it impossible to test them and verify they are correct. This commit fixes this by converting them into a proper mini-app. In a subsequent commit, tests will be added to verify that the source code works as expected (and guard against regressions). Fixes #34314 PR Close #34362
|
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. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #34314
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Closes: #34314