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.

Sign up for GitHub

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

Jump to bottom

Revisit config and defaultConfig as getters. #92

Closed
otherdaniel opened this issue May 18, 2021 · 1 comment
Closed

Revisit config and defaultConfig as getters. #92

otherdaniel opened this issue May 18, 2021 · 1 comment
Assignees
@otherdaniel
Labels
sanitizer-api issues with the API

Comments

@otherdaniel
Copy link
Collaborator

.config() and .defaultConfig() are currently methods, why they should be properties, or at least, use property syntax. (.config). A getter would be a way to accomplish this.

Per https://github.com/WICG/sanitizer-api/pull/80/files#r616408446
(via whatwg/webidl#981)
(via #77)
(via w3ctag/design-reviews#619)
@otherdaniel otherdaniel added the sanitizer-api issues with the API label May 18, 2021
@otherdaniel otherdaniel self-assigned this May 18, 2021
@otherdaniel otherdaniel mentioned this issue Jun 30, 2021
Merged
pull bot pushed a commit to Yannic/chromium that referenced this issue Jul 7, 2021
Revert "[Sanitizer] Implement config and defaultConfig as attributes."
5fa877b 
This reverts commit e3d90a7.

Reason for revert: Consistent failures on multiple buildbots:
  crbug.com/1226882

Original change's description:
> [Sanitizer] Implement config and defaultConfig as attributes.
>
> This follows spec discussion at github.com/WICG/sanitizer-api/issues/92
>
> Bug: 1213893
> Change-Id: I9b7102fecd7f896b5cec160592a5d6bd60c16716
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2996826
> Commit-Queue: Daniel Vogelheim <[email protected]>
> Reviewed-by: Yifan Luo <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#898870}

Bug: 1213893
Change-Id: Icb5ca18bd24e97cd27b4a8f71f96c3994397785d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3010215
Bot-Commit: Rubber Stamper <[email protected]>
Commit-Queue: Nicolas Ouellet-Payeur <[email protected]>
Owners-Override: Nicolas Ouellet-Payeur <[email protected]>
Cr-Commit-Position: refs/heads/master@{#898927}
blueboxd pushed a commit to blueboxd/chromium-legacy that referenced this issue Jul 12, 2021
@otherdaniel
[Sanitizer API] Rename config() to getConfiguration().
66474c9 
Refs:
- github.com/WICG/sanitizer-api/issues/92
- github.com/WICG/sanitizer-api/issues/107
- WICG/sanitizer-api#108

Bug: 1213893
Change-Id: Ibbc89e2678107f835517af9743e5a1eeca3911d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3015576
Reviewed-by: Yifan Luo <[email protected]>
Commit-Queue: Daniel Vogelheim <[email protected]>
Cr-Commit-Position: refs/heads/master@{#900407}
@otherdaniel
Copy link
Collaborator Author

This was resolved with PR #104, and then reverted with #108 based on discussion at #107. I'm unaware of any issues with the current solution, so I think this issue is now obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@otherdaniel otherdaniel

Labels
sanitizer-api issues with the API
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@otherdaniel