Skip to content

The report having a property named report is confusing #42

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
RByers opened this issue May 30, 2017 · 3 comments
Closed

The report having a property named report is confusing #42

RByers opened this issue May 30, 2017 · 3 comments
Assignees

Comments

@RByers
Copy link
Collaborator

RByers commented May 30, 2017

This is a silly thing but it's been bugging me. Mostly the spec uses the term report to refer to the larger thing (eg. that has a type) and body to refer to the the type-specific data structure. But then the exposed property name for the body is "report".

This leads to awkward observer code like report.report.

Can we just rename the property to body to match the spec terminology?

Alternately, since we likely want a WebIDL definition for each report type (at least those exposed via #29), we could recast the spec in terms of those types instead of a generic notion of "any objection which can be serialized to JSON" (which itself is problematic), and then the body could just be contained directly in the report via interface inheritance:

interface ReportBase
{
    readonly attribute string type;
};

interface DeprecationReport : ReportBase
{
    readonly attribute string message;
};

@mikewest @juliatuttle

@RByers RByers changed the title The report having a member report is potentially confusing The report having a property named report is confusing May 30, 2017
@mikewest
Copy link
Member

Renaming the property to body sounds perfectly fine to me. I think I like that clear separation between the generic reporting API stuff, and the stuff specific to a specific thing that's being reported. It also resolves the question that will inevitably crop up when a feature has a property that collides with a generic reporting API property (e.g. assume your example had deprecation "types").

RByers added a commit to RByers/reporting that referenced this issue May 31, 2017
@RByers RByers self-assigned this May 31, 2017
@RByers
Copy link
Collaborator Author

RByers commented May 31, 2017

Thanks SGTM. PTAL at #43

@RByers
Copy link
Collaborator Author

RByers commented May 31, 2017

By the way, I think we might still want to define the reports in terms of WebIDL interfaces instead of just "JSON serializable ECMAScript objects", but we can discuss that separately as part of the spec work for #29.

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

No branches or pull requests

2 participants