Skip to content

[CP][beta channel] 2a8f3d5 - Update dartdoc revision to 2.0.0 #46983

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

Closed
jcollins-g opened this issue Aug 24, 2021 · 10 comments
Closed

[CP][beta channel] 2a8f3d5 - Update dartdoc revision to 2.0.0 #46983

jcollins-g opened this issue Aug 24, 2021 · 10 comments
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. cherry-pick-approved Label for approved cherrypick request merge-to-stable

Comments

@jcollins-g
Copy link
Contributor

commit(s) to merge: 2a8f3d5

merge instructions: https://dart-review.googlesource.com/c/sdk/+/211085. (Patch had conflict in DEPS, CL resolves)

What is the issue: Dartdoc's revision at beta HEAD contained a crash bug in the generic typedef support introduced when the new lookup code was switched on by default.

What is the fix: The proposed fix is to update the revision of dartdoc shipped with 2.14 to the current stable release of dartdoc, 2.0.0.

Why cherrypick: Having these bugs fixed would prevent reports of crash bugs in dartdoc that can be corrected by pub global activate dartdoc.

Risk: The cherry pick to update dartdoc can't be done to only incorporate the bugfixes as per usual cherry-pick policy. It is therefore possible that changing revisions will introduce new problems of some sort. To mitigate this I have built and tested a beta SDK with the compiled dartdoc to smoke test it by hand, testing it on dartdoc's "test package" and the SDK itself.

Link to original issue(s): dart-lang/dartdoc#2740 dart-lang/dartdoc#2755

/cc @kevmoo @mit-mit @whesse @athomas @vsmenon @franklinyow

@jcollins-g jcollins-g added area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. merge-to-beta cherry-pick-review Issue that need cherry pick triage to approve labels Aug 24, 2021
@jcollins-g
Copy link
Contributor Author

To elaborate on the risk, the most important downstream users of dartdoc use pub global activate dartdoc or other similar means to grab the latest stable dartdoc rather than the version compiled in the SDK, including pub.dev and Flutter. So this is lower risk than it otherwise might be.

@devoncarew
Copy link
Member

Can you elaborate on the impact? We use dartdoc to publish api.dart.dev, api.flutter.dev, and the api docs on pub.dev? From my read of the issue, none of these would be affected by this (they either don't hit the issue or take from the pub published dartdoc). Is that correct? The only people that would be affected are the ones that run dartdoc manually from the sdk?

@jcollins-g
Copy link
Contributor Author

The dartdoc in the SDK is used to generate the docs on api.dart.dev for that SDK version. Otherwise, folks manually running dartdoc from the SDK will be the only people impacted by this change.

@sigmundch
Copy link
Member

For those curious, this is the actual diff on the dartdoc side that would come from updating the DEPS reference:
dart-lang/dartdoc@5f39ec6...a4ca86f

And this is the original commit that fixed the underlying issue in dartdoc: dart-lang/dartdoc@ba16d63

@devoncarew
Copy link
Member

This is approved, but for the first round of cherry-picks after the stable. I'll update w/ the cp-approved label after we've shipped (and keep the current 'cherry-pick-review' label until then).

@athomas
Copy link
Member

athomas commented Aug 30, 2021

FYI, api.dart.dev docs are built with the dartdoc in the Dart SDK. Currently, they seem to work https://api.dart.dev/beta so perhaps the SDK isn't affected by this issue.

@athomas
Copy link
Member

athomas commented Aug 30, 2021

@devoncarew Why merge-to-stable? Do we want to patch 2.13.4 (currently on stable)? Or do we want this to into 2.14 (currently on beta)?

I should have read the last comment more carefully, if we want to CP onto 2.14 in the first 2.14 hotfix rather than merge this before the release merge-to-stable is the correct label.

@devoncarew
Copy link
Member

@athomas - yup, that was my thinking. This is essentially merge-to-stable for the stable that's not yet out :)

@devoncarew devoncarew added cherry-pick-approved Label for approved cherrypick request and removed cherry-pick-review Issue that need cherry pick triage to approve labels Sep 9, 2021
@devoncarew
Copy link
Member

Approved! @athomas - note that this is now a cp to stable, not beta. Not sure if that will be an issue for you. If so, I'm sure @jcollins-g could provide a new cp.

@athomas
Copy link
Member

athomas commented Sep 15, 2021

Merged to stable in 3300f32 (2.14.2).

@athomas athomas closed this as completed Sep 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. cherry-pick-approved Label for approved cherrypick request merge-to-stable
Projects
None yet
Development

No branches or pull requests

4 participants