Skip to content
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

docs: update docs to use HttpClientModule instead of HttpModule #22727

Closed

Conversation

agussman
Copy link
Contributor

Updated most examples to use HttpClientModule instead of deprecated HttpModule

fix #19280

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: #19280

What is the new behavior?

Updated doc examples to use HttpClientModule instead of old HttpModule.

Does this PR introduce a breaking change?

[ ] Yes
[ ] No

Other information

@ngbot
Copy link

ngbot bot commented Mar 20, 2018

Hi @agussman! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

2 similar comments
@ngbot
Copy link

ngbot bot commented Mar 20, 2018

Hi @agussman! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@ngbot
Copy link

ngbot bot commented Mar 20, 2018

Hi @agussman! This PR has merge conflicts due to recent upstream merges.
Please help to unblock it by resolving these conflicts. Thanks!

@jenniferfell
Copy link
Contributor

@agussman : Hi. Now that v6 is released, we're getting around to closing some open loops. I apologize for letting this sit for so long. If you're still interested, we'd be thrilled to have you contribute this fix. It's a bit out-of-date, as I think some PRs were merged that fixed some of the HTTPModule issues you were addressing. I think, however, there are still some areas that aren't fixed. I made a few notes on the original issue.

Is there anything I can do to help you make this contribution? Thanks!

@agussman
Copy link
Contributor Author

agussman commented May 4, 2018

@jenniferfell : Yes, I would like to contribute the PR. What is required from my end?

@jenniferfell
Copy link
Contributor

@agussman: Fantastic! The first thing I recommend doing is resolving conflicts between what we have in master and what you have in this PR. We just released v6 yesterday, so it's a good time. If you're familiar with github, there's nothing special about doing this with our example and doc files.

For some tips about how we merge changes from master into our doc-examples branches, my team started to collect some information:
https://github.com/ExpertSupport/doc-workflow/wiki/Daily-Startup (see step 2 especially)
https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/

@agussman agussman force-pushed the update-docs-with-HttpClientModule branch from 7e11568 to 8ba2159 Compare May 6, 2018 02:59
@agussman
Copy link
Contributor Author

agussman commented May 6, 2018

Hi @jenniferfell : I merged in the latest master, resolved conflicts, and updated the PR. Can you take a look and let me know if there are any issues that would prevent it from being merged? Thanks!

@jenniferfell jenniferfell added target: patch This PR is targeted for the next patch release comp: examples labels May 8, 2018
@jenniferfell jenniferfell requested a review from alxhub May 9, 2018 18:08
@agussman agussman force-pushed the update-docs-with-HttpClientModule branch from 8ba2159 to 53a1348 Compare May 10, 2018 03:34
Updated most examples to use HttpClientModule instead of deprecated HttpModule

fix angular#19280
@agussman agussman force-pushed the update-docs-with-HttpClientModule branch from 53a1348 to 4d33477 Compare May 15, 2018 01:26
@IgorMinar IgorMinar added the action: merge The PR is ready for merge by the caretaker label May 15, 2018
@agussman
Copy link
Contributor Author

@IgorMinar Thank you for the code review!

@jenniferfell Anything else needed from me?

@IgorMinar IgorMinar added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label May 22, 2018
@IgorMinar
Copy link
Contributor

caretaker note: the failing CI tests seems like an unrelated flake. I restarted the job, but if that doesn't help please check on the failure and merge this PR.

@agussman I don't think so. I think the CI failure is unrelated to your change, so let's wait for the caretaker to investigate.

vicb pushed a commit that referenced this pull request May 30, 2018
Updated most examples to use HttpClientModule instead of deprecated HttpModule

fix #19280

PR Close #22727
@vicb vicb closed this in 3ed7fc6 May 30, 2018
vicb added a commit that referenced this pull request May 30, 2018
vicb added a commit that referenced this pull request May 30, 2018
@vicb vicb reopened this May 30, 2018
@vicb vicb removed action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels May 30, 2018
@vicb vicb added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label May 30, 2018
@vicb
Copy link
Contributor

vicb commented May 30, 2018

@agussman there is an issue with the changes causing the CI to fail. Could you please take a look.

@agussman
Copy link
Contributor Author

@vicb: Happy to take a look, but per @IgorMinar's comment the CI failures don't appear to be related to these changes.

@gkalpak
Copy link
Member

gkalpak commented May 31, 2018

@agussman, there are still files that are importing from @angular/http (e.g. importing Http) and some of them are intentiinally doing so. You need to ensure that the examples e2e tests pass with any changes you make.

(We also nee to make sure that guide text is update (if necessary).)

@brandonroberts
Copy link
Contributor

@agussman will you rebase on master?

@brandonroberts brandonroberts self-assigned this Jul 5, 2018
@jenniferfell jenniferfell added the state: community Someone from the Angular community is working on this issue or submitted this PR label Jul 23, 2018
@brandonroberts
Copy link
Contributor

Closing in favor of smaller PRs already merged and in process

@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 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews cla: yes state: community Someone from the Angular community is working on this issue or submitted this PR target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation straddles HttpModule and HttpClientModule
8 participants