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

Small changes #11

Merged
merged 3 commits into from
Nov 10, 2022
Merged

Small changes #11

merged 3 commits into from
Nov 10, 2022

Conversation

mosuem
Copy link
Member

@mosuem mosuem commented Nov 9, 2022

While I also like composition over inheritance, I thought this would be a good use of polymorphism. This is absolutely optional, just an idea.

Copy link
Member

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

Looks good! This is an improvement over the multiple if statements I had. One suggestion about a class rename.

packages/corpus/lib/report.dart Outdated Show resolved Hide resolved
buf.writeln('https://api.dart.dev/dart-${reportTarget.name}/'
'dart-${reportTarget.name}-library.html');
} else {
buf.writeln(reportTarget.description);
buf.writeln((reportTarget as PackageTarget).description);
Copy link
Member

Choose a reason for hiding this comment

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

If description were a nullable property of ReportTarget we could avoid this cast.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added the type check to clarify, but I would rather not make this a property of ReportTarget as a DartLibraryTarget really has no description (so far).

@mosuem mosuem merged commit 3a211a1 into core_libs Nov 10, 2022
@mosuem mosuem deleted the suggestions_to_core_libs branch November 10, 2022 08:32
devoncarew added a commit that referenced this pull request Nov 10, 2022
* add support for dart: libraries

* commit two sample reports

* Small changes (#11)

* Small changes

* Rename class

* Inline variable

Co-authored-by: Moritz <mosum@google.com>
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