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(upgrading): add link to upgrading snippets #2788

Closed
wants to merge 2 commits into from

Conversation

ST-DDT
Copy link
Member

@ST-DDT ST-DDT commented Apr 7, 2024

Currently the docs don't contain a hint how to upgrade.

https://next.fakerjs.dev/guide/upgrading.html

This PR adds a link to the individual snippets until we merge them into one big file.

https://deploy-preview-2788.fakerjs.dev/guide/upgrading.html

@ST-DDT ST-DDT added c: docs Improvements or additions to documentation p: 1-normal Nothing urgent labels Apr 7, 2024
@ST-DDT ST-DDT added this to the v9.0 milestone Apr 7, 2024
@ST-DDT ST-DDT requested review from a team April 7, 2024 14:33
@ST-DDT ST-DDT self-assigned this Apr 7, 2024
Copy link

netlify bot commented Apr 7, 2024

Deploy Preview for fakerjs ready!

Name Link
🔨 Latest commit 4e04c67
🔍 Latest deploy log https://app.netlify.com/sites/fakerjs/deploys/66151609bbb4160008dd987d
😎 Deploy Preview https://deploy-preview-2788.fakerjs.dev
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.95%. Comparing base (b505a70) to head (4e04c67).

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2788      +/-   ##
==========================================
- Coverage   99.96%   99.95%   -0.01%     
==========================================
  Files        2971     2971              
  Lines      212553   212553              
  Branches      947      949       +2     
==========================================
- Hits       212475   212466       -9     
- Misses         78       87       +9     

see 2 files with indirect coverage changes

@matthewmayer
Copy link
Contributor

I'd rather concatenate all the snippets into the files. The alpha is useful for looking for mistakes in the docs as well as mistakes in the code, and that's easier with a concatenated file.

@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Apr 9, 2024
@xDivisionByZerox
Copy link
Member

I'd rather concatenate all the snippets into the files. The alpha is useful for looking for mistakes in the docs as well as mistakes in the code, and that's easier with a concatenated file.

Not sure about that. The reason we opted for these separate migration files was the idea to write a comprehensive migration files that makes sense in it's structure (order of section, grouping of related changes, etc). We would assentially do all this work twice by already concatinating all files.

That being said, I'm not sure about simply puting a link to the snippet directory - the UX feels really bad.

@matthewmayer
Copy link
Contributor

We could do a rough version just by deciding on a sensible order and noting down the order of files for future reference

cat xxxx.md yyyy.md zzzz.md > ../upgrading.md

@ST-DDT
Copy link
Member Author

ST-DDT commented Apr 10, 2024

IMO we could just merge the migration guide files together, since we already have most breaking stuff already. So before any release me merge the snippets into the main file.
AFAICT we already completed most of the migration categories, so any additional migration task would end up in a new section anyway and not likely affect the existing ones.
Or that being said, we could stop the separate migration files at this point.

@ST-DDT ST-DDT closed this Apr 11, 2024
@ST-DDT ST-DDT deleted the docs/upgrading/link-snippets branch April 11, 2024 17:17
@ST-DDT ST-DDT removed the s: needs decision Needs team/maintainer decision label Apr 11, 2024
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants