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

♻️ Simplify log/assert code and use core assert #34061

Merged
merged 4 commits into from
Apr 28, 2021

Conversation

rcebulko
Copy link
Contributor

Moves detection of error.associatedElement from log#prepareError_ to error-reporting where it is used. Simplifies log#assert method in preparation for adding type assertion helpres to core.

@amp-owners-bot amp-owners-bot bot requested a review from dvoytenko April 28, 2021 17:16
@amp-owners-bot
Copy link

Hey @jridgewell! These files were changed:

src/core/assert.js

@rcebulko rcebulko requested review from samouri and removed request for dvoytenko April 28, 2021 17:17
Copy link
Contributor Author

@rcebulko rcebulko left a comment

Choose a reason for hiding this comment

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

OWNERS bump

src/log.js Outdated Show resolved Hide resolved
@rcebulko rcebulko merged commit 8d97947 into ampproject:main Apr 28, 2021
@rcebulko rcebulko deleted the no-assoc-el branch April 28, 2021 21:11
rochapablo pushed a commit to rochapablo/amphtml that referenced this pull request Aug 30, 2021
* Report error in core if reporting fn defined

* Use base assertion fn directly in log

* Move associatedElement from log to error reporting

* Use concat in apply
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue: restricted paths eslint allowlist I2I: src/core directory with no runtime dependencies
3 participants