Skip to content

Document arbitrary SDK-like things as Dart SDKs. #1665

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
merged 40 commits into from
Apr 12, 2018

Conversation

jcollins-g
Copy link
Contributor

@jcollins-g jcollins-g commented Apr 11, 2018

Fixes #1617. Enabled by the config object refactoring that's ongoing, started in #1661 and #1662.

Dartdoc now allows you to specify a --sdk-dir as input. And it will automatically detect if the CWD or --input-dir is inside a built SDK or something that looks like it, like this subdirectory of the SDK repo and switch to SDK mode.

The flags allow you to document a package against a different version of the SDK than the one you're running dartdoc with, which could be useful on the pub site or for testing purposes. You can also document Windows SDKs from a Linux box, Mac SDKs from a Windows box, and so on.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Apr 11, 2018
@jcollins-g jcollins-g requested review from devoncarew and kevmoo April 11, 2018 18:53
@kevmoo
Copy link
Member

kevmoo commented Apr 11, 2018

Excited to have this feature, but letting @devoncarew give formal approval

@jcollins-g jcollins-g requested a review from pq April 11, 2018 21:29
Copy link
Member

@pq pq left a comment

Choose a reason for hiding this comment

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

Substantially, LGTM w/ a few wee nits.

bin/dartdoc.dart Outdated
abbr: 'h', negatable: false, help: 'Show command help.');
parser.addFlag('hide-sdk-text',
help:
"Drop all text for SDK components. Helpful for integration tests for dartdoc, probably not useful for anything else.",
Copy link
Member

Choose a reason for hiding this comment

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

Single-quotes here too?

Copy link
Member

Choose a reason for hiding this comment

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

There's a lint for that 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kevmoo Indeed, but there's apparently no way to automatically apply the lint resolution to all the lints that show up when I enable it, and doing Double-Click Alt-Enter Down-arrow Down-arrow Enter several hundred times does not sound like my idea of fun.

In the meantime I'll try not to make things worse. Done.

bin/dartdoc.dart Outdated
parser.addOption('ambiguous-reexport-scorer-min-confidence',
help:
'Minimum scorer confidence to suppress warning on ambiguous reexport.',
defaultsTo: "0.1",
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps stick with single-quotes for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

help: 'Prints out progress JSON maps. One entry per line.',
defaultsTo: false,
negatable: true);
'Group libraries from the same package in the libraries sidebar. (deprecated, ignored)',
Copy link
Member

Choose a reason for hiding this comment

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

=> (Deprecated, ignored)

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

use-categories was marked deprecated, ignored back in #1641 because the meaning of "category" changed and we no longer require this flag to turn on what was category mode. Dartdoc now knows whether to do that by context and we just ignore this flag if it gets passed in.

The diff looks weird because I sorted the options here to better keep track of them visually, as I'm going to be doing major surgery on them soon.

@@ -151,5 +151,5 @@ void main() {
dart_bear.allClasses.map((cls) => cls.name).contains('Bear'), isTrue);
expect(p.packageMap["Dart"].publicLibraries, hasLength(3));
});
}, timeout: new Timeout.factor(4));
}, timeout: new Timeout.factor(8));
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, did things get slower or were you seeing flakes before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flakes.

@jcollins-g jcollins-g merged commit 3b0bc62 into master Apr 12, 2018
@jcollins-g jcollins-g deleted the config-object-passthrough+generator+sdkDir branch April 12, 2018 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA check succeeded.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants