Skip to content

[Records] dartdoc implemention #3122

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
itsjustkevin opened this issue Aug 19, 2022 · 10 comments
Closed

[Records] dartdoc implemention #3122

itsjustkevin opened this issue Aug 19, 2022 · 10 comments
Assignees
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug

Comments

@itsjustkevin
Copy link

No description provided.

@bwilkerson bwilkerson added the P1 A high priority bug; for example, a single project is unusable or has many test failures label Aug 29, 2022
@srawlins
Copy link
Member

srawlins commented Nov 1, 2022

Largely complete in #3233; leaving open for remaining tasks.

@vsmenon
Copy link
Member

vsmenon commented Feb 6, 2023

@srawlins - do we have anything to do here?

@srawlins
Copy link
Member

srawlins commented Feb 6, 2023

Hmm, yikes, I have no idea what I meant by "leaving open for remaining tasks." This may not be complete.

@jcollins-g jcollins-g self-assigned this Feb 15, 2023
@jcollins-g jcollins-g added the type-enhancement A request for a change that isn't a bug label Feb 15, 2023
@jcollins-g
Copy link
Contributor

@srawlins One task that I can see is that there's no bit in test_package_experiments that allows us to check the integration of the feature. Adding that may also help me discover what was meant by "remaining tasks", if any others exist.

@srawlins
Copy link
Member

You could add that. I have been writing all new tests as unit tests instead of using the testing/ folder.

@jcollins-g
Copy link
Contributor

@srawlins. Saw that. That's probably for the best for most cases. Having something browsable helps make sure we didn't miss anything as I don't trust my brain to render all the test HTML as well as an actual web browser.

@jcollins-g
Copy link
Contributor

jcollins-g commented Feb 15, 2023

Two things so far I've found that need to be fixed:

  • The record type rendering is not very legible, and while it looks Dart-like would not be able to reproduce the type.

image

The type for this variable should be written out as (int, double, c: String, d: int) to match how a type for this variable would be declared.

  • We are not preserving the typedef name in the case of records. While that matches the underlying implementation it is probably not in line with user expectations/friendliness.

Given source:

RecordTypedef<double> fromParameterizedRecordType = (3.5, 10, "A string");

I would expect RecordTypedef<double> to be the type of fromParameterizedRecordType, with linking.
Contrast (double, int, String), or the current Record(double $0, int $1, String $2).

@srawlins
Copy link
Member

As long as we stay in-line with the handling of other typedefs, SG.

@jcollins-g
Copy link
Contributor

As long as we stay in-line with the handling of other typedefs, SG.

The second item (preserving typedef names) seems the most clearly in line with other typedef handling, so doing that first. I'll do some checking to make sure my plan for the first item remains consistent.

@jcollins-g
Copy link
Contributor

This should be complete with #3344; any problems discovered later can be separate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants