Skip to content

Remove non-null assertions on common/http - #48241

Closed
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:feature/fix-24571-common-http
Closed

Remove non-null assertions on common/http#48241
JeanMeche wants to merge 1 commit into
angular:mainfrom
JeanMeche:feature/fix-24571-common-http

Conversation

@JeanMeche

@JeanMeche JeanMeche commented Nov 27, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch 5 times, most recently from f9f6b59 to d37fff7 Compare November 27, 2022 16:20
@JeanMeche JeanMeche changed the title Feature/fix 24571 common http remove non-null assertions on common/http & common/directives Nov 27, 2022
@JeanMeche JeanMeche changed the title remove non-null assertions on common/http & common/directives Eemove non-null assertions on common/http & common/directives Nov 27, 2022
@JeanMeche JeanMeche changed the title Eemove non-null assertions on common/http & common/directives Remove non-null assertions on common/http & common/directives Nov 27, 2022
@jessicajaniuk jessicajaniuk added the area: common Issues related to APIs in the @angular/common package label Dec 9, 2022
@ngbot ngbot Bot added this to the Backlog milestone Dec 9, 2022
pkozlowski-opensource
pkozlowski-opensource reviewed Dec 12, 2022
View reviewed changes
Comment thread packages/common/src/directives/ng_for_of.ts Outdated
pkozlowski-opensource
pkozlowski-opensource reviewed Dec 12, 2022
View reviewed changes
Comment thread packages/common/src/directives/ng_plural.ts Outdated
pkozlowski-opensource
pkozlowski-opensource requested changes Dec 12, 2022
View reviewed changes

@pkozlowski-opensource pkozlowski-opensource left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@JeanMeche thnx for the PR. There is quite a bit going on in there and the main difficulty in reviewing those PRs is that there are many changes in several packages.
Could you please split your PR into 2 and:

  • separate common/http changes from other changes in a separate PR?
  • have a separate commit / PR for each directive ?

@pkozlowski-opensource pkozlowski-opensource added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: common/http Issues related to HTTP and HTTP Client labels Dec 12, 2022
@JeanMeche JeanMeche mentioned this pull request Dec 13, 2022
Closed
2 tasks done
@angular-robot angular-robot Bot added the area: build & ci Related the build and CI infrastructure of the project label Dec 13, 2022
@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch from 860bf9b to d37fff7 Compare December 13, 2022 17:36
@angular-robot angular-robot Bot removed the area: build & ci Related the build and CI infrastructure of the project label Dec 13, 2022
@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch 2 times, most recently from 9ebe990 to bcd5a08 Compare December 13, 2022 17:44
@JeanMeche JeanMeche changed the title Remove non-null assertions on common/http & common/directives Remove non-null assertions on common/http Dec 13, 2022
@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch from bcd5a08 to c8245a3 Compare January 10, 2023 15:44
@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch from c8245a3 to f8dbc47 Compare February 3, 2023 13:45
@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch from f8dbc47 to 7b828e7 Compare February 27, 2023 18:51
@JeanMeche
refactor(common): remove non-null assertions
40fa0fb 
As part of angular#24571, removing all non-null assertions from common/http
@JeanMeche
JeanMeche force-pushed the feature/fix-24571-common-http branch from 7b828e7 to 40fa0fb Compare February 27, 2023 19:18
@JeanMeche

JeanMeche commented Feb 27, 2023 •
edited
Loading

Copy link
Copy Markdown
Member Author

I've updated the PR as part of the work to tackle #24571.

How breaking is the change for HttpResponseBase.type in G3 ?

This was referenced Feb 27, 2023
Closed
Closed
Closed
Closed
@JeanMeche JeanMeche closed this May 1, 2023
@JeanMeche
JeanMeche deleted the feature/fix-24571-common-http branch May 1, 2023 14:07
@angular-automatic-lock-bot

angular-automatic-lock-bot Bot commented Jun 1, 2023

Copy link
Copy Markdown

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

Reviewers

@pkozlowski-opensource pkozlowski-opensource Awaiting requested review from pkozlowski-opensource

Assignees

No one assigned

Labels

action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: common/http Issues related to HTTP and HTTP Client area: common Issues related to APIs in the @angular/common package

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Post strictPropertyInitialization flag flip cleanup

3 participants

@JeanMeche @pkozlowski-opensource @jessicajaniuk