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

pass in url for site #30

Merged
merged 3 commits into from
Dec 12, 2014
Merged

pass in url for site #30

merged 3 commits into from
Dec 12, 2014

Conversation

keertip
Copy link
Collaborator

@keertip keertip commented Dec 12, 2014

Directory out;
Set<LibraryElement> libraries = new Set();
HtmlGenerator generator;

DartDoc(this._rootDir, this._excludes);
DartDoc(this._rootDir, this._excludes, this._url);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should _url be optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could make it optional, but then would have to check whether to pass it in or not. Don't know which is the better approach here.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could say that URL is a property of the backend. From an API perspective, URL really only belongs in HtmlGenerator.

How is this DartDoc class intended to be used? I don't think DartDoc class should have a reference to HtmlGenerator. Instead:

  • Create one or more generators, and configure them
  • Give the list of generators to DartDoc

Thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, the url is for the HtmlGenerator. Like the idea of separation and configuring the generator based on user args. Will refactor next time around.

keertip added a commit that referenced this pull request Dec 12, 2014
@keertip keertip merged commit 901086a into dart-lang:master Dec 12, 2014
@keertip keertip deleted the site branch December 12, 2014 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants