Skip to content

Universal: Integrate CLI wiki material and copy edit#25752

Closed
jbogarthyde wants to merge 1 commit intoangular:masterfrom
jbogarthyde:jb-cli-universal
Closed

Universal: Integrate CLI wiki material and copy edit#25752
jbogarthyde wants to merge 1 commit intoangular:masterfrom
jbogarthyde:jb-cli-universal

Conversation

@jbogarthyde
Copy link
Copy Markdown
Contributor

@jbogarthyde jbogarthyde commented Aug 30, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Added more succinct CLI-oriented info at beginning as a summary, made the existing ToH example more specifically tutorial-style.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@jbogarthyde jbogarthyde added comp: docs area: server Issues related to server-side rendering target: patch This PR is targeted for the next patch release labels Aug 30, 2018
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Aug 30, 2018

You can preview ebbee7f at https://pr25752-ebbee7f.ngbuilds.io/.

@CaerusKaru
Copy link
Copy Markdown
Member

CaerusKaru commented Aug 31, 2018

Not sure if this PR is the appropriate place for this, but we recently removed the Webpack part of the Universal build process in universal and universal-starter. cc @vikerman @Toxicable @IgorMinar for guidance.

@jbogarthyde
Copy link
Copy Markdown
Contributor Author

jbogarthyde commented Aug 31, 2018

This is exactly the right place to correct the current doc :-)
Can you mark up the exact parts of the doc that should be removed or changed?
And also, the content should get a complete technical review from everyone involved, if possible.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Aug 31, 2018

You can preview a171d8a at https://pr25752-a171d8a.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 3, 2018

You can preview 6649b56 at https://pr25752-6649b56.ngbuilds.io/.

@jenniferfell jenniferfell added the action: review The PR is still awaiting reviews from at least one requested reviewer label Sep 4, 2018
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 10, 2018

You can preview e2100dd at https://pr25752-e2100dd.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 11, 2018

You can preview bb88b3b at https://pr25752-bb88b3b.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 13, 2018

You can preview 7e7aeb5 at https://pr25752-7e7aeb5.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 14, 2018

You can preview eb81296 at https://pr25752-eb81296.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 14, 2018

You can preview 95a8e18 at https://pr25752-95a8e18.ngbuilds.io/.

@jbogarthyde jbogarthyde removed the request for review from vikerman September 18, 2018 17:50
@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 19, 2018

You can preview fc1b2f3 at https://pr25752-fc1b2f3.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 20, 2018

You can preview 1015933 at https://pr25752-1015933.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Sep 21, 2018

You can preview a236105 at https://pr25752-a236105.ngbuilds.io/.

@vikerman
Copy link
Copy Markdown
Contributor

vikerman commented Oct 11, 2018

We don't need to include the webpack CLI info as it's not needed.

jenniferfell
jenniferfell approved these changes Oct 11, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@jenniferfell jenniferfell left a comment

Choose a reason for hiding this comment

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

I only looked at the overall structure in the right nav. LGTM except the two things we talked about in slack.

Comment thread aio/content/guide/universal.md Outdated
Copy link
Copy Markdown
Contributor

@jenniferfell jenniferfell Oct 11, 2018

Choose a reason for hiding this comment

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

From our slack...to avoid duplicate heading, maybe "Modify your app and configuration"

Comment thread aio/content/guide/universal.md Outdated
Copy link
Copy Markdown
Contributor

@jenniferfell jenniferfell Oct 11, 2018

Choose a reason for hiding this comment

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

In the preview that I looked at, this heading isn't rendering correctly

Copy link
Copy Markdown
Contributor Author

@jbogarthyde jbogarthyde Oct 11, 2018

Choose a reason for hiding this comment

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

Fixed it already, and the md is now correct. Still not rendering though - don't know what else to do to fix it.

@jenniferfell
Copy link
Copy Markdown
Contributor

jenniferfell commented Oct 11, 2018

@vikerman Just to confirm, these changes also apply to v6, yes?

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Oct 11, 2018

You can preview c48c4d5 at https://pr25752-c48c4d5.ngbuilds.io/.

@vikerman
Copy link
Copy Markdown
Contributor

vikerman commented Oct 12, 2018

@jenniferfell - Yes. Applies to V6 as well.

@vikerman
Copy link
Copy Markdown
Contributor

vikerman commented Oct 12, 2018

Approved. Restarting the flaky tests

@vikerman vikerman added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 12, 2018
@ngbot
Copy link
Copy Markdown

ngbot Bot commented Oct 12, 2018

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci/circleci: integration_test" is failing
    failure status "continuous-integration/travis-ci/pr" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

@vikerman
Copy link
Copy Markdown
Contributor

vikerman commented Oct 12, 2018

@jbogarthyde - Can you please rebase the PR? Tests continue to fail (clearly unrelated to this PR but would be better to get it passing cleanly before merge)

@jbogarthyde jbogarthyde force-pushed the jb-cli-universal branch 2 times, most recently from 90f12c8 to 6b1c429 Compare October 12, 2018 21:40
@jbogarthyde
Copy link
Copy Markdown
Contributor Author

jbogarthyde commented Oct 12, 2018

Rebased and squashed

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Oct 12, 2018

You can preview 90f12c8 at https://pr25752-90f12c8.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Oct 12, 2018

You can preview 6b1c429 at https://pr25752-6b1c429.ngbuilds.io/.

@mary-poppins
Copy link
Copy Markdown

mary-poppins commented Oct 15, 2018

You can preview de25370 at https://pr25752-de25370.ngbuilds.io/.

mhevery pushed a commit that referenced this pull request Oct 15, 2018
@jbogarthyde @mhevery
docs: update process for cli tool and restructure doc (#25752)
a251374 
PR Close #25752
@mhevery mhevery closed this in 5183bbf Oct 15, 2018
@alan-agius4 alan-agius4 mentioned this pull request Oct 16, 2018
Closed
3 tasks
@jbogarthyde jbogarthyde deleted the jb-cli-universal branch October 16, 2018 16:47
This was referenced Oct 17, 2018
Closed
Closed
Closed
Closed
@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot Bot commented Sep 14, 2019

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 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@vikerman vikerman vikerman approved these changes

@jenniferfell jenniferfell jenniferfell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: server Issues related to server-side rendering cla: yes target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

v7.0

Development

Successfully merging this pull request may close these issues.

6 participants

@jbogarthyde @mary-poppins @CaerusKaru @vikerman @jenniferfell @googlebot