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

Print rustdoc rendering warnings all the time #45324

Merged
merged 1 commit into from Oct 20, 2017

Conversation

Projects
None yet
7 participants
@GuillaumeGomez
Member

GuillaumeGomez commented Oct 16, 2017

@QuietMisdreavus

This comment has been minimized.

Show comment
Hide comment
@QuietMisdreavus

QuietMisdreavus Oct 16, 2017

Member

cc #44229

IMO it's way too early for this, like shouldn't we be following that process? The next step is to print the warnings by default (instead of just when --enable-commonmark is given) and wait for a stabilization cycle. Has that happened yet?

Member

QuietMisdreavus commented Oct 16, 2017

cc #44229

IMO it's way too early for this, like shouldn't we be following that process? The next step is to print the warnings by default (instead of just when --enable-commonmark is given) and wait for a stabilization cycle. Has that happened yet?

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Oct 16, 2017

Member

Hum... I thought that the next step was to make the pulldown renderer the default one in the next cycle and to print the warnings by default (which makes sense). Didn't understood that it was two steps?

Member

GuillaumeGomez commented Oct 16, 2017

Hum... I thought that the next step was to make the pulldown renderer the default one in the next cycle and to print the warnings by default (which makes sense). Didn't understood that it was two steps?

@QuietMisdreavus

This comment has been minimized.

Show comment
Hide comment
@QuietMisdreavus

QuietMisdreavus Oct 16, 2017

Member

As far as i understand the intended process, we were supposed to warn people about the rendering changes without changing the rendering output. By switching the default renderer like this, we're warning them about the changes while we mess up their docs, which is what we set out to avoid by adding all this overhead.

Member

QuietMisdreavus commented Oct 16, 2017

As far as i understand the intended process, we were supposed to warn people about the rendering changes without changing the rendering output. By switching the default renderer like this, we're warning them about the changes while we mess up their docs, which is what we set out to avoid by adding all this overhead.

@GuillaumeGomez GuillaumeGomez changed the title from Switch markdown default renderer to pulldown to Print rustdoc rendering warnings all the time Oct 16, 2017

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Oct 16, 2017

Member

Updated.

Member

GuillaumeGomez commented Oct 16, 2017

Updated.

@nrc nrc referenced this pull request Oct 16, 2017

Closed

Tracking issue (Rustdoc): hoedown -> pulldown migration #44229

10 of 10 tasks complete
@QuietMisdreavus

This comment has been minimized.

Show comment
Hide comment
@QuietMisdreavus

QuietMisdreavus Oct 16, 2017

Member

Much better! Now we just print the warnings all the time, and begin our stability cycle for real.

Member

QuietMisdreavus commented Oct 16, 2017

Much better! Now we just print the warnings all the time, and begin our stability cycle for real.

@michaelwoerister

This comment has been minimized.

Show comment
Hide comment
@michaelwoerister

michaelwoerister Oct 16, 2017

Contributor

r? @steveklabnik

We discussed this in the dev-tools meeting. It seems like the next logical step. Thanks for the PR, @GuillaumeGomez!

Contributor

michaelwoerister commented Oct 16, 2017

r? @steveklabnik

We discussed this in the dev-tools meeting. It seems like the next logical step. Thanks for the PR, @GuillaumeGomez!

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik Oct 17, 2017

Member

@bors: r+

thank you!

Member

steveklabnik commented Oct 17, 2017

@bors: r+

thank you!

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 17, 2017

Contributor

📌 Commit 11b2b08 has been approved by steveklabnik

Contributor

bors commented Oct 17, 2017

📌 Commit 11b2b08 has been approved by steveklabnik

@ollie27

This comment has been minimized.

Show comment
Hide comment
@ollie27

ollie27 Oct 17, 2017

Contributor

Is there any reason that these warnings aren't emitted when using rustdoc on plain markdown files?

Contributor

ollie27 commented Oct 17, 2017

Is there any reason that these warnings aren't emitted when using rustdoc on plain markdown files?

@GuillaumeGomez

This comment has been minimized.

Show comment
Hide comment
@GuillaumeGomez

GuillaumeGomez Oct 17, 2017

Member

@ollie27: No idea. It should though... I think it's a separate issue. Can you open an issue so I can look into it later on please?

Member

GuillaumeGomez commented Oct 17, 2017

@ollie27: No idea. It should though... I think it's a separate issue. Can you open an issue so I can look into it later on please?

@ollie27

This comment has been minimized.

Show comment
Hide comment
@ollie27

ollie27 Oct 18, 2017

Contributor

done: #45365

Contributor

ollie27 commented Oct 18, 2017

done: #45365

@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 20, 2017

Contributor

⌛️ Testing commit 11b2b08 with merge 95272a0...

Contributor

bors commented Oct 20, 2017

⌛️ Testing commit 11b2b08 with merge 95272a0...

bors added a commit that referenced this pull request Oct 20, 2017

Auto merge of #45324 - GuillaumeGomez:switch-default-markdown-rendere…
…r, r=steveklabnik

Print rustdoc rendering warnings all the time

r? @rust-lang/dev-tools
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors Oct 20, 2017

Contributor

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing 95272a0 to master...

Contributor

bors commented Oct 20, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing 95272a0 to master...

@bors bors merged commit 11b2b08 into rust-lang:master Oct 20, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@GuillaumeGomez GuillaumeGomez deleted the GuillaumeGomez:switch-default-markdown-renderer branch Oct 21, 2017

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
Is it really time? Have our months, no, *years* of suffering come to an end? Are we finally able to cast off the pall of Hoedown? The weight which has dragged us down for so long?

-----

So, timeline for those who need to catch up:

* Way back in December 2016, [we decided we wanted to switch out the markdown renderer](rust-lang#38400). However, this was put on hold because the build system at the time made it difficult to pull in dependencies from crates.io.
* A few months later, in March 2017, [the first PR was done, to switch out the renderers entirely](rust-lang#40338). The PR itself was fraught with CI and build system issues, but eventually landed.
* However, not all was well in the Rustdoc world. During the PR and shortly after, we noticed [some differences in the way the two parsers handled some things](rust-lang#40912), and some of these differences were major enough to break the docs for some crates.
* A couple weeks afterward, [Hoedown was put back in](rust-lang#41290), at this point just to catch tests that Pulldown was "spuriously" running. This would at least provide some warning about spurious tests, rather than just breaking spontaneously.
* However, the problems had created enough noise by this point that just a few days after that, [Hoedown was switched back to the default](rust-lang#41431) while we came up with a solution for properly warning about the differences.
* That solution came a few weeks later, [as a series of warnings when the HTML emitted by the two parsers was semantically different](rust-lang#41991). But that came at a cost, as now rustdoc needed proc-macro support (the new crate needed some custom derives farther down its dependency tree), and the build system was not equipped to handle it at the time. It was worked on for three months as the issue stumped more and more people.
  * In that time, [bootstrap was completely reworked](rust-lang#43059) to change how it ordered compilation, and [the method by which it built rustdoc would change](rust-lang#43482), as well. This allowed it to only be built after stage1, when proc-macros would be available, allowing the "rendering differences" PR to finally land.
  * The warnings were not perfect, and revealed a few [spurious](rust-lang#44368) [differences](rust-lang#45421) between how we handled the renderers.
  * Once these were handled, [we flipped the switch to turn on the "rendering difference" warnings all the time](rust-lang#45324), in October 2017. This began the "warning cycle" for this change, and landed in stable in 1.23, on 2018-01-04.
  * Once those warnings hit stable, and after a couple weeks of seeing whether we would get any more reports than what we got from sitting on nightly/beta, [we switched the renderers](rust-lang#47398), making Pulldown the default but still offering the option to use Hoedown.

And that brings us to the present. We haven't received more new issues from this in the meantime, and the "switch by default" is now on beta. Our reasoning is that, at this point, anyone who would have been affected by this has run into it already.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
Is it really time? Have our months, no, *years* of suffering come to an end? Are we finally able to cast off the pall of Hoedown? The weight which has dragged us down for so long?

-----

So, timeline for those who need to catch up:

* Way back in December 2016, [we decided we wanted to switch out the markdown renderer](rust-lang#38400). However, this was put on hold because the build system at the time made it difficult to pull in dependencies from crates.io.
* A few months later, in March 2017, [the first PR was done, to switch out the renderers entirely](rust-lang#40338). The PR itself was fraught with CI and build system issues, but eventually landed.
* However, not all was well in the Rustdoc world. During the PR and shortly after, we noticed [some differences in the way the two parsers handled some things](rust-lang#40912), and some of these differences were major enough to break the docs for some crates.
* A couple weeks afterward, [Hoedown was put back in](rust-lang#41290), at this point just to catch tests that Pulldown was "spuriously" running. This would at least provide some warning about spurious tests, rather than just breaking spontaneously.
* However, the problems had created enough noise by this point that just a few days after that, [Hoedown was switched back to the default](rust-lang#41431) while we came up with a solution for properly warning about the differences.
* That solution came a few weeks later, [as a series of warnings when the HTML emitted by the two parsers was semantically different](rust-lang#41991). But that came at a cost, as now rustdoc needed proc-macro support (the new crate needed some custom derives farther down its dependency tree), and the build system was not equipped to handle it at the time. It was worked on for three months as the issue stumped more and more people.
  * In that time, [bootstrap was completely reworked](rust-lang#43059) to change how it ordered compilation, and [the method by which it built rustdoc would change](rust-lang#43482), as well. This allowed it to only be built after stage1, when proc-macros would be available, allowing the "rendering differences" PR to finally land.
  * The warnings were not perfect, and revealed a few [spurious](rust-lang#44368) [differences](rust-lang#45421) between how we handled the renderers.
  * Once these were handled, [we flipped the switch to turn on the "rendering difference" warnings all the time](rust-lang#45324), in October 2017. This began the "warning cycle" for this change, and landed in stable in 1.23, on 2018-01-04.
  * Once those warnings hit stable, and after a couple weeks of seeing whether we would get any more reports than what we got from sitting on nightly/beta, [we switched the renderers](rust-lang#47398), making Pulldown the default but still offering the option to use Hoedown.

And that brings us to the present. We haven't received more new issues from this in the meantime, and the "switch by default" is now on beta. Our reasoning is that, at this point, anyone who would have been affected by this has run into it already.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
Is it really time? Have our months, no, *years* of suffering come to an end? Are we finally able to cast off the pall of Hoedown? The weight which has dragged us down for so long?

-----

So, timeline for those who need to catch up:

* Way back in December 2016, [we decided we wanted to switch out the markdown renderer](rust-lang#38400). However, this was put on hold because the build system at the time made it difficult to pull in dependencies from crates.io.
* A few months later, in March 2017, [the first PR was done, to switch out the renderers entirely](rust-lang#40338). The PR itself was fraught with CI and build system issues, but eventually landed.
* However, not all was well in the Rustdoc world. During the PR and shortly after, we noticed [some differences in the way the two parsers handled some things](rust-lang#40912), and some of these differences were major enough to break the docs for some crates.
* A couple weeks afterward, [Hoedown was put back in](rust-lang#41290), at this point just to catch tests that Pulldown was "spuriously" running. This would at least provide some warning about spurious tests, rather than just breaking spontaneously.
* However, the problems had created enough noise by this point that just a few days after that, [Hoedown was switched back to the default](rust-lang#41431) while we came up with a solution for properly warning about the differences.
* That solution came a few weeks later, [as a series of warnings when the HTML emitted by the two parsers was semantically different](rust-lang#41991). But that came at a cost, as now rustdoc needed proc-macro support (the new crate needed some custom derives farther down its dependency tree), and the build system was not equipped to handle it at the time. It was worked on for three months as the issue stumped more and more people.
  * In that time, [bootstrap was completely reworked](rust-lang#43059) to change how it ordered compilation, and [the method by which it built rustdoc would change](rust-lang#43482), as well. This allowed it to only be built after stage1, when proc-macros would be available, allowing the "rendering differences" PR to finally land.
  * The warnings were not perfect, and revealed a few [spurious](rust-lang#44368) [differences](rust-lang#45421) between how we handled the renderers.
  * Once these were handled, [we flipped the switch to turn on the "rendering difference" warnings all the time](rust-lang#45324), in October 2017. This began the "warning cycle" for this change, and landed in stable in 1.23, on 2018-01-04.
  * Once those warnings hit stable, and after a couple weeks of seeing whether we would get any more reports than what we got from sitting on nightly/beta, [we switched the renderers](rust-lang#47398), making Pulldown the default but still offering the option to use Hoedown.

And that brings us to the present. We haven't received more new issues from this in the meantime, and the "switch by default" is now on beta. Our reasoning is that, at this point, anyone who would have been affected by this has run into it already.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 16, 2018

Remove hoedown from rustdoc
Is it really time? Have our months, no, *years* of suffering come to an end? Are we finally able to cast off the pall of Hoedown? The weight which has dragged us down for so long?

-----

So, timeline for those who need to catch up:

* Way back in December 2016, [we decided we wanted to switch out the markdown renderer](rust-lang#38400). However, this was put on hold because the build system at the time made it difficult to pull in dependencies from crates.io.
* A few months later, in March 2017, [the first PR was done, to switch out the renderers entirely](rust-lang#40338). The PR itself was fraught with CI and build system issues, but eventually landed.
* However, not all was well in the Rustdoc world. During the PR and shortly after, we noticed [some differences in the way the two parsers handled some things](rust-lang#40912), and some of these differences were major enough to break the docs for some crates.
* A couple weeks afterward, [Hoedown was put back in](rust-lang#41290), at this point just to catch tests that Pulldown was "spuriously" running. This would at least provide some warning about spurious tests, rather than just breaking spontaneously.
* However, the problems had created enough noise by this point that just a few days after that, [Hoedown was switched back to the default](rust-lang#41431) while we came up with a solution for properly warning about the differences.
* That solution came a few weeks later, [as a series of warnings when the HTML emitted by the two parsers was semantically different](rust-lang#41991). But that came at a cost, as now rustdoc needed proc-macro support (the new crate needed some custom derives farther down its dependency tree), and the build system was not equipped to handle it at the time. It was worked on for three months as the issue stumped more and more people.
  * In that time, [bootstrap was completely reworked](rust-lang#43059) to change how it ordered compilation, and [the method by which it built rustdoc would change](rust-lang#43482), as well. This allowed it to only be built after stage1, when proc-macros would be available, allowing the "rendering differences" PR to finally land.
  * The warnings were not perfect, and revealed a few [spurious](rust-lang#44368) [differences](rust-lang#45421) between how we handled the renderers.
  * Once these were handled, [we flipped the switch to turn on the "rendering difference" warnings all the time](rust-lang#45324), in October 2017. This began the "warning cycle" for this change, and landed in stable in 1.23, on 2018-01-04.
  * Once those warnings hit stable, and after a couple weeks of seeing whether we would get any more reports than what we got from sitting on nightly/beta, [we switched the renderers](rust-lang#47398), making Pulldown the default but still offering the option to use Hoedown.

And that brings us to the present. We haven't received more new issues from this in the meantime, and the "switch by default" is now on beta. Our reasoning is that, at this point, anyone who would have been affected by this has run into it already.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment