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

fix(docs-infra): add deprecated-api-item class to remaining deprecated items #34192

Closed

Conversation

@ajitsinghkaler
Copy link
Contributor

ajitsinghkaler commented Dec 2, 2019

Fixes #31455

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?
It adds strikethrough to deprecate items

  • 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?

Deprecated items are not striked through

Issue Number: #31455

What is the new behavior?

Deprecated Items are stroked through

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@ajitsinghkaler ajitsinghkaler requested a review from angular/docs-infra as a code owner Dec 2, 2019
@googlebot googlebot added the cla: yes label Dec 2, 2019
@ajitsinghkaler

This comment has been minimized.

Copy link
Contributor Author

ajitsinghkaler commented Dec 2, 2019

@petebacondarwin I have made the rest of the changes you can have a look

@ngbot ngbot bot modified the milestone: Backlog Dec 2, 2019
@kapunahelewong kapunahelewong added this to Pending - Top of backlog in docs Dec 2, 2019
@kapunahelewong kapunahelewong moved this from Pending - Top of backlog to In Progress in docs Dec 2, 2019
@gkalpak gkalpak added the aio: preview label Dec 3, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 3, 2019

@gkalpak
gkalpak approved these changes Dec 3, 2019
Copy link
Member

gkalpak left a comment

Looks reasonable. Do we know of any examples where these changes actually make a difference (i.e. deprecated stuff that would not show up correctly without these changes)?

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Dec 3, 2019

BTW, the commit message type should be docs-infra, not docs. should be fix(docs-infra): ....

@ajitsinghkaler

This comment has been minimized.

Copy link
Contributor Author

ajitsinghkaler commented Dec 3, 2019

@gkalpak I'm attaching images for reference where it removes deprecated items. I've given both examples where it was not done earlier and now it's done. https://pr34192-45f36de.ngbuilds.io/api/core/Testability you can check this link for details.

Angular - Testability (3)
Angular - Testability (2)
Angular - Testability (1)
Angular - Testability
Angular - Testability (4)

@ajitsinghkaler ajitsinghkaler force-pushed the ajitsinghkaler:deprecated-api-item branch from 45f36de to 0aac71f Dec 3, 2019
@ajitsinghkaler ajitsinghkaler requested a review from gkalpak Dec 3, 2019
@ajitsinghkaler

This comment has been minimized.

Copy link
Contributor Author

ajitsinghkaler commented Dec 3, 2019

@gkpalak I've made the change please have a look at them.

@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 3, 2019

@gkalpak gkalpak changed the title docs: add deprecated-api-item class to remaining deprecated items fix(docs-infra): add deprecated-api-item class to remaining deprecated items Dec 3, 2019
@gkalpak
gkalpak approved these changes Dec 3, 2019
Copy link
Member

gkalpak left a comment

Thx, @ajitsinghkaler! The link and screenshots are really helpful 👌

I wonder if it might be better to strikethrough the method-table header (instead of the signature):
deprecated-method

@ajitsinghkaler ajitsinghkaler force-pushed the ajitsinghkaler:deprecated-api-item branch from 0aac71f to 82317d6 Dec 4, 2019
@ajitsinghkaler

This comment has been minimized.

Copy link
Contributor Author

ajitsinghkaler commented Dec 4, 2019

@gkalpak So, should I add strikethrough to the headings or it is just fine. For me both are fine but what do you think is correct because right now as I see in the documentation we strike through the signature not the headings. Please suggest the correct way.

@ajitsinghkaler ajitsinghkaler requested a review from gkalpak Dec 4, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 4, 2019

@ajitsinghkaler ajitsinghkaler force-pushed the ajitsinghkaler:deprecated-api-item branch from 82317d6 to 2c0e074 Dec 4, 2019
@mary-poppins

This comment has been minimized.

Copy link

mary-poppins commented Dec 4, 2019

@ajitsinghkaler

This comment has been minimized.

Copy link
Contributor Author

ajitsinghkaler commented Dec 4, 2019

@gkalpak I also don't know why these tests are failing

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Dec 4, 2019

The more I think about it, the more I don't like striking through the signature.
So, unless someone objects, I believe that (if we want to strike through something) we should be striking through the headers.

@petebacondarwin, wdyt?

@ajitsinghkaler

This comment has been minimized.

Copy link
Contributor Author

ajitsinghkaler commented Dec 5, 2019

@gkalpak okay, thanks for all the guidance. I'll put strkethrough in the heading once @petebacondarwin confirms that we should do it.

@petebacondarwin

This comment has been minimized.

Copy link
Member

petebacondarwin commented Dec 5, 2019

One problem, I think.
The signatures are per overload and there might be scenarios where one overload is deprecated while others are not…

@gkalpak

This comment has been minimized.

Copy link
Member

gkalpak commented Dec 5, 2019

Agh...good point. OK, then. Let's keep the strike-through on the signature for now. We can revisit later.

@gkalpak
gkalpak approved these changes Dec 5, 2019
AndrewKushnir added a commit that referenced this pull request Dec 5, 2019
@ajitsinghkaler ajitsinghkaler deleted the ajitsinghkaler:deprecated-api-item branch Dec 11, 2019
josephperrott added a commit to josephperrott/angular that referenced this pull request Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.