Skip to content

docs: strikethrough deprecated methods on api index #2067

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

Merged

Conversation

matthewmayer
Copy link
Contributor

@matthewmayer matthewmayer commented Apr 17, 2023

Strikes through deprecated methods on API index page so its easier to see the non-deprecated methods

Example:
Screenshot 2023-04-16 at 20 12 59

@matthewmayer matthewmayer requested a review from a team April 17, 2023 03:48
@matthewmayer matthewmayer requested a review from a team as a code owner April 17, 2023 03:48
@codecov
Copy link

codecov bot commented Apr 17, 2023

Codecov Report

Merging #2067 (941afa3) into next (a001ac5) will increase coverage by 0.00%.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             next    #2067   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files        2535     2535           
  Lines      242202   242202           
  Branches     1292     1295    +3     
=======================================
+ Hits       241258   241268   +10     
+ Misses        917      907   -10     
  Partials       27       27           

see 1 file with indirect coverage changes

Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

I somewhat like it, but I'm not sure whether the user easily understands that the methods there are deprecated/what the strike-through should mean.

Also I'm not sure whether this adds any kind of benefit. If you notice a method doing what you search for then you click on it. If it happens to be deprecated, you will read the deprecation hint pointing to the right method.

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent labels Apr 17, 2023
@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Apr 17, 2023
@matthewmayer
Copy link
Contributor Author

The idea is if you are skimming looking for the right method you can focus on the ones that are non-deprecated.

@ST-DDT
Copy link
Member

ST-DDT commented Apr 20, 2023

Team Decision

  • We want this.

@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Apr 20, 2023
@ST-DDT ST-DDT requested review from a team April 20, 2023 16:22
ST-DDT
ST-DDT previously approved these changes Apr 20, 2023
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@ST-DDT ST-DDT requested review from a team April 20, 2023 17:45
@Shinigami92 Shinigami92 dismissed stale reviews from ST-DDT and xDivisionByZerox via 4907c71 April 20, 2023 18:24
@ST-DDT ST-DDT merged commit bf67a21 into faker-js:next Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: docs Improvements or additions to documentation p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants