docs: fix code style #45527

Piyush132000 · 2022-04-05T03:20:53Z

I have removed unnecessary code from the components & I initialize the form in the OnInit function (life cycle). I think initialization of forms and getting data from the service must be done inside ngOnInit.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines:https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Update the code style of Product-list & Shipping component.
Removed Unnecessary code.

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

What is the current behavior?

Same behavior as previous

Issue Number: N/A

What is the new behavior?

NA

Does this PR introduce a breaking change?

Yes
No

Other information

AlirezaEbrahimkhani · 2022-04-07T11:18:57Z

@Piyush132000
Tnx
but can you please update the commit message to more closely follow angular commit message guidelines? 🙏
Somthing like this :

docs: fix code style

Updated Product list & shipping component code style

To update, your last commit message you can use this command git commit --amend and then force push your branch
remember you do not need to add another commit message just edit your last commit message. 👍

Piyush132000 · 2022-04-07T11:34:39Z

@Piyush132000 Tnx but can you please update the commit message to more closely follow angular commit message guidelines? 🙏 Somthing like this :

docs: fix code style
Updated Product list & shipping component code style

To update, your last commit message you can use this command git commit --amend and then force push your branch remember you do not need to add another commit message just edit your last commit message. 👍

@AlirezaEbrahimkhani thanks sir and I updated the commit message.

jessicajaniuk · 2022-04-11T17:25:40Z

@Piyush132000 this has some lint and test failures that should be quick to resolve. If you can get them addressed, this is otherwise green and should be quick to land.

Piyush132000 · 2022-04-13T02:55:24Z

@Piyush132000 this has some lint and test failures that should be quick to resolve. If you can get them addressed, this is otherwise green and should be quick to land.

Sorry for inconvenience, can you please help me how can I remove these failure?

jessicajaniuk · 2022-04-13T15:40:54Z

@Piyush132000 The test_aio failure looks like a simple linting issue where you need a space after a : in one of your files. The test_docs_examples test looks like an issue with one of the types used in the template. You can always click on "Details" in the failed tests listed below in the PR here to see the details of the failed tests.

Piyush132000 · 2022-04-13T16:02:32Z

@Piyush132000 The test_aio failure looks like a simple linting issue where you need a space after a : in one of your files. The test_docs_examples test looks like an issue with one of the types used in the template. You can always click on "Details" in the failed tests listed below in the PR here to see the details of the failed tests.

Thanks @jessicajaniuk

Piyush132000 · 2022-04-15T03:10:17Z

Hii @jessicajaniuk, I have fixed lint-related issues.

Please verify.

jessicajaniuk · 2022-04-15T16:09:25Z

@Piyush132000 Looks like the lint issues are fixed, but you still have some test_docs_examples issues. You can see them here.

Piyush132000 · 2022-04-20T03:56:03Z

@Piyush132000 Looks like the lint issues are fixed, but you still have some test_docs_examples issues. You can see them here.

Sorry for the inconvenience, @jessicajaniuk I tried but was not able to fix the failed tests.

Piyush132000 · 2022-04-28T02:35:53Z

@Piyush132000 Looks like the lint issues are fixed, but you still have some test_docs_examples issues. You can see them here.

Sorry for the inconvenience, @jessicajaniuk I tried but was not able to fix the failed tests.

Hii @jessicajaniuk can you please help me more so I can fix these failures.

jessicajaniuk · 2022-04-28T11:04:24Z

aio/content/examples/getting-started/src/app/shipping/shipping.component.ts


-  shippingCosts = this.cartService.getShippingPrices();
+  shippingCosts: {  type: string , price: number}[] = [];


This is what's breaking the tests. shippingCosts should be an observable since the cartService getShippingPrices method returns an observable. The error in the tests is this:

error TS2740: Type 'Observable<{ type: string; price: number; }[]>' is missing the following properties from type '{ type: string; price: number; }[]': length, pop, push, concat, and 27 more

This is essentially saying there's a type mismatch between what you've specified here, which is just a simple array of objects, and the actual return value, which is an Observable. This needs to be resolved to fix the tests.

gkalpak

Thx for making these improvements, @Piyush132000 👍
I left a couple of suggestions.

aio/content/examples/getting-started/src/app/shipping/shipping.component.ts

Piyush132000 · 2022-05-05T03:46:30Z

Hii @jessicajaniuk @gkalpak please check my PR again.

Thanks

mary-poppins · 2022-05-05T10:05:57Z

You can preview 052aa44 at https://pr45527-052aa44.ngbuilds.io/.
You can preview 228febc at https://pr45527-228febc.ngbuilds.io/.
You can preview 2068330 at https://pr45527-2068330.ngbuilds.io/.
You can preview c7fb01d at https://pr45527-c7fb01d.ngbuilds.io/.
You can preview 96281dc at https://pr45527-96281dc.ngbuilds.io/.
You can preview cc445b9 at https://pr45527-cc445b9.ngbuilds.io/.

gkalpak

Just a couple of formatting suggestions. Otherwise lgtm 👍

aio/content/start/start-data.md

aio/content/examples/getting-started/src/app/shipping/shipping.component.ts

mary-poppins · 2022-05-05T12:10:42Z

You can preview 0da8181 at https://pr45527-0da8181.ngbuilds.io/.

mary-poppins · 2022-05-05T12:30:33Z

You can preview 16c31df at https://pr45527-16c31df.ngbuilds.io/.

mary-poppins · 2022-05-22T16:18:17Z

You can preview 4cb7ec8 at https://pr45527-4cb7ec8.ngbuilds.io/.

Piyush132000 · 2022-05-22T16:30:53Z

Hi @jessicajaniuk, @gkalpak Please review my PR.

Thanks

gkalpak

LGTM 🚀
Thx, @Piyush132000 👍

Reviewed-for: global-docs-approvers

gkalpak · 2022-05-23T11:17:41Z

aio/content/start/start-data.md

+1. Define a `shippingCosts` property that sets the `shippingCosts` property using the `getShippingPrices()` method from the `CartService`.
+   Initialize the `shippingCosts` property inside `ngOnInit()` method.


Nit: The indentation does not match the other items here, but not worth blocking this PR over that.

Comments have been addressed (i.e. CI is green).

jessicajaniuk

LGTM 🍪

Thanks for your work and persistence in getting this green, @Piyush132000! 🎉

alxhub

Hello @Piyush132000

Sorry to kick this back, but when I was going to merge this I noticed this PR is composed of 4 commits, all of which have the same generic title and no description.

Please squash these commits into one, and add a short description describing why we're making these changes.

mary-poppins · 2022-05-24T05:18:20Z

You can preview 77979d9 at https://pr45527-77979d9.ngbuilds.io/.

I have removed unnecessary code from the components & i initialize the form in OnInit function (life cycle). I think initilization of forms and getting data from service must be done inside ngOnInit.

mary-poppins · 2022-05-27T02:20:56Z

You can preview 425a619 at https://pr45527-425a619.ngbuilds.io/.

Piyush132000 · 2022-06-04T05:12:38Z

Hii @jessicajaniuk, @gkalpak, and @alxhub any changes required to merge this PR to main? I already squashed the commits.

Thanks

jessicajaniuk · 2022-06-14T19:56:51Z

This PR was merged into the repository by commit 9ce733a.

I have removed unnecessary code from the components & i initialize the form in OnInit function (life cycle). I think initilization of forms and getting data from service must be done inside ngOnInit. PR Close #45527

Piyush132000 · 2022-06-15T03:52:25Z

Thanks, @jessicajaniuk @gkalpak @alxhub

angular-automatic-lock-bot · 2022-07-16T16:05:53Z

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.}

pullapprove bot requested a review from AndrewKushnir April 5, 2022 03:21

Piyush132000 force-pushed the code-style-update-and-refactoring branch from 052aa44 to 228febc Compare April 7, 2022 11:33

Piyush132000 changed the title ~~Fix: docs code style~~ docs: fix code style Apr 7, 2022

jessicajaniuk added the comp: docs label Apr 7, 2022

ngbot bot added this to the Backlog milestone Apr 7, 2022

Piyush132000 force-pushed the code-style-update-and-refactoring branch from 2068330 to c7fb01d Compare April 13, 2022 16:33

jessicajaniuk previously requested changes Apr 28, 2022

View reviewed changes

Piyush132000 force-pushed the code-style-update-and-refactoring branch from 87195b8 to 225b5cd Compare April 29, 2022 04:15

gkalpak requested changes Apr 29, 2022

View reviewed changes

aio/content/examples/getting-started/src/app/shipping/shipping.component.ts Outdated Show resolved Hide resolved

aio/content/examples/getting-started/src/app/shipping/shipping.component.ts Outdated Show resolved Hide resolved

Piyush132000 force-pushed the code-style-update-and-refactoring branch 2 times, most recently from 8e061a1 to cc445b9 Compare May 5, 2022 03:31

gkalpak added the aio: preview label May 5, 2022

gkalpak requested changes May 5, 2022

View reviewed changes

Piyush132000 force-pushed the code-style-update-and-refactoring branch from 1f871ef to 4cb7ec8 Compare May 22, 2022 16:09

gkalpak approved these changes May 23, 2022

View reviewed changes

gkalpak removed request for AndrewKushnir and jessicajaniuk May 23, 2022 11:23

jessicajaniuk approved these changes May 23, 2022

View reviewed changes

alxhub added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: merge The PR is ready for merge by the caretaker labels May 23, 2022

alxhub requested changes May 23, 2022

View reviewed changes

Piyush132000 force-pushed the code-style-update-and-refactoring branch from 4cb7ec8 to 77979d9 Compare May 24, 2022 05:09

Piyush132000 requested a review from alxhub May 27, 2022 02:09

docs: fix code style

425a619

I have removed unnecessary code from the components & i initialize the form in OnInit function (life cycle). I think initilization of forms and getting data from service must be done inside ngOnInit.

Piyush132000 force-pushed the code-style-update-and-refactoring branch from 77979d9 to 425a619 Compare May 27, 2022 02:11

alxhub approved these changes Jun 14, 2022

View reviewed changes

jessicajaniuk added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jun 14, 2022

jessicajaniuk closed this in 9ce733a Jun 14, 2022

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 16, 2022

Apr	MAY	Jun
	21
2024	2025	2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: fix code style #45527

docs: fix code style #45527

Piyush132000 commented Apr 5, 2022 •

edited

Loading

AlirezaEbrahimkhani commented Apr 7, 2022

Piyush132000 commented Apr 7, 2022

jessicajaniuk commented Apr 11, 2022

Piyush132000 commented Apr 13, 2022

jessicajaniuk commented Apr 13, 2022 •

edited

Loading

Piyush132000 commented Apr 13, 2022

Piyush132000 commented Apr 15, 2022

jessicajaniuk commented Apr 15, 2022

Piyush132000 commented Apr 20, 2022 •

edited

Loading

Piyush132000 commented Apr 28, 2022

jessicajaniuk Apr 28, 2022

gkalpak left a comment

Piyush132000 commented May 5, 2022

mary-poppins commented May 5, 2022

gkalpak left a comment

mary-poppins commented May 5, 2022

mary-poppins commented May 5, 2022

mary-poppins commented May 22, 2022

Piyush132000 commented May 22, 2022

gkalpak left a comment •

edited

Loading

gkalpak May 23, 2022

jessicajaniuk left a comment

alxhub left a comment

mary-poppins commented May 24, 2022

mary-poppins commented May 27, 2022

Piyush132000 commented Jun 4, 2022 •

edited

Loading

jessicajaniuk commented Jun 14, 2022

Piyush132000 commented Jun 15, 2022

angular-automatic-lock-bot bot commented Jul 16, 2022


		shippingCosts = this.cartService.getShippingPrices();
		shippingCosts: { type: string , price: number}[] = [];

		1. Define a `shippingCosts` property that sets the `shippingCosts` property using the `getShippingPrices()` method from the `CartService`.
		Initialize the `shippingCosts` property inside `ngOnInit()` method.

docs: fix code style #45527

docs: fix code style #45527

Conversation

Piyush132000 commented Apr 5, 2022 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

AlirezaEbrahimkhani commented Apr 7, 2022

Piyush132000 commented Apr 7, 2022

jessicajaniuk commented Apr 11, 2022

Piyush132000 commented Apr 13, 2022

jessicajaniuk commented Apr 13, 2022 • edited Loading

Piyush132000 commented Apr 13, 2022

Piyush132000 commented Apr 15, 2022

jessicajaniuk commented Apr 15, 2022

Piyush132000 commented Apr 20, 2022 • edited Loading

Piyush132000 commented Apr 28, 2022

jessicajaniuk Apr 28, 2022

Choose a reason for hiding this comment

gkalpak left a comment

Choose a reason for hiding this comment

Piyush132000 commented May 5, 2022

mary-poppins commented May 5, 2022

gkalpak left a comment

Choose a reason for hiding this comment

mary-poppins commented May 5, 2022

mary-poppins commented May 5, 2022

mary-poppins commented May 22, 2022

Piyush132000 commented May 22, 2022

gkalpak left a comment • edited Loading

Choose a reason for hiding this comment

gkalpak May 23, 2022

Choose a reason for hiding this comment

jessicajaniuk left a comment

Choose a reason for hiding this comment

alxhub left a comment

Choose a reason for hiding this comment

mary-poppins commented May 24, 2022

mary-poppins commented May 27, 2022

Piyush132000 commented Jun 4, 2022 • edited Loading

jessicajaniuk commented Jun 14, 2022

Piyush132000 commented Jun 15, 2022

angular-automatic-lock-bot bot commented Jul 16, 2022

Piyush132000 commented Apr 5, 2022 •

edited

Loading

jessicajaniuk commented Apr 13, 2022 •

edited

Loading

Piyush132000 commented Apr 20, 2022 •

edited

Loading

gkalpak left a comment •

edited

Loading

Piyush132000 commented Jun 4, 2022 •

edited

Loading