-
Notifications
You must be signed in to change notification settings - Fork 22.5k
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
Convert Web/JavaScript to markdown #6936
Conversation
move *.html to *.md
convert content to md
I’d like to express a strong preference here to not hard-wrap the Markdown output to any particular character width — that is, I’d strongly prefer we preserve the existing line lengths unwrapped, as they are in the current HTML sources. But I see that this patch as currently written causes all the Markdown output to be hard-wrapped to 80 characters. I don’t know if that completely intentional — and is one of the explicit design decisions already documented somewhere — or is instead just a default setting in the tooling that we’re not overriding; but regardless, I would like as for consideration that we don’t do that re-wrapping in Markdown output but instead preserve the original line lengths un-wrapped. |
I believe this is being done by Prettier, and that it's possible to configure it. We haven't explicitly discussed line length but I'm happy to not wrap. |
@fiji-flo Awesome ^^^ - that's my experience of git too. FWIW in the files view it is being shown as a delete/recreate, but I hope that as long as we avoid the squash it will work properly "in the end". @wbamberg @sideshowbarker That said, I also much prefer we do not arbitrarily change the wrapping too (FWIW my personal preference tends to be line breaking on sentences - though I have not good argument for this other than it makes some manual translation software easier to use). |
These I believe are all the current open PRs that touch /javascript https://github.com/mdn/content/pull/2861/files It would be good to merge as many of these as we can in the next day or so - we already have conflicts on this PR so we'll need to redo it anyway. I think we should decouple https://github.com/mdn/content/pull/2861/files and https://github.com/mdn/content/pull/5197/files from this PR, by converting the files in those PRs to Markdown once they are ready. |
Let us NOT hard-wrap text, ever, please and thank you. |
I couldn't find the config to suggest against, but let's use Prettier's default Further reading:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked at every page in detail but have looked at quite a few, and some of the more complicated ones, and they all look great.
Would it be possible to change the line-wrapping config to use "preserve", as Daniel requested in #6936 (comment) ?
Also inevitably there are conflicts now, so this will need to be regenerated anyway.
But with those changes I think we should merge this.
I'm not in on Monday, so we could aim to merge on Tuesday? We should also ask people not to merge PRs to the JS tree in between this PR being updated and it being merged.
I don't know if we want any further review from anyone, @Rumyra or @Elchi3 or anyone else in the core review crew? The previous PR: #5193 had quite a bit of scrutiny and this is almost identical (the only change I know of is mdn/yari#4221).
Core js reviewers are: Mentioning so we know to hold off on merges when told 👍 |
This PR now shows merge conflicts 😿 |
Closing in favour of #7092 |
These are two commits and we must not squash merge.
git
is smart an decides it's cheaper to consider this adelete
+add
operation instead ofrename
+modify