Skip to content

refactor: begin extracting per-package data out from Package #1622

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 4 commits into from
Mar 2, 2018

Conversation

jcollins-g
Copy link
Contributor

First part of #1610. In order to implement categories and other parameters on a per-package basis, rather than globally, Dartdoc needs a data structure that is truly per-package. The Package class has long had a TODO for a major refactor and at least some parts of that really can't be avoided now without significantly adding to technical debt.

In this first PR, only libraries are made per-package and the bare minimum of initialization and referencing has been moved over -- most objects still are talking to PackageGraph when they are going to reference Package in the future.

@googlebot googlebot added the cla: yes Google CLA check succeeded. label Feb 28, 2018
@kevmoo
Copy link
Member

kevmoo commented Mar 1, 2018

Thanks for running on this, @jcollins-g !

pubspec.lock Outdated
@@ -415,4 +415,4 @@ packages:
source: hosted
version: "2.1.13"
sdks:
dart: ">=2.0.0-dev <=2.0.0-dev.30.0"
dart: ">=2.0.0-dev <=2.0.0-edge.0d5cf900b021bf5c9fa593ffa12b15bcd1cc5fe0"
Copy link
Member

Choose a reason for hiding this comment

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

I really don't know the consequences of committing this. I don't know how pub would know to compare two edge versions. If this is an issue, we should have a travis check here so that an edge constraint can't be committed.

Copy link
Member

Choose a reason for hiding this comment

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

these cause zero issues. Just a bit weird to look at...

@@ -4497,12 +4508,12 @@ class Package extends Canonicalization with Nameable, Warnable {
}
}

class PackageCategory implements Comparable<PackageCategory> {
class Package implements Comparable<Package> {
Copy link
Member

Choose a reason for hiding this comment

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

👍

// Library objects serving as entry points for documentation.
final List<Library> _libraries = [];

class PackageGraph extends Canonicalization with Nameable, Warnable {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I would call it a PackageGraph myself (PackageGroup? Project?), but whatever you're comfortable with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eventually, this object will represent the collection of Packages and their cross-links via dependencies being documented. I admit that PackageGraph is more ambition than reality at this point.

@kevmoo
Copy link
Member

kevmoo commented Mar 2, 2018

@jcollins-g – make sure you delete the .dart_tool directory and update .gitignore! - https://groups.google.com/a/dartlang.org/forum/#!topic/misc/JICjwA7AOro

@kevmoo
Copy link
Member

kevmoo commented Mar 2, 2018

er, don't delete it – but git rm -rf .dart_tool

@kevmoo
Copy link
Member

kevmoo commented Mar 2, 2018

@jcollins-g – those were always generated, just into .pub. Now they are put in .dart_tool.

Update the .gitignore and you'll be good....

@jcollins-g
Copy link
Contributor Author

Yep already was figuring that out. #1624 has the gitignore change.

@jcollins-g jcollins-g merged commit 4645500 into master Mar 2, 2018
@jcollins-g jcollins-g deleted the dartdoc-package-extract-beginning branch March 2, 2018 21:45
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