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
Initial smatr-ads-net #27382
Initial smatr-ads-net #27382
Conversation
I am not the correct reviewer for Ads related code. @powerivq is more appropriate. Unassigning myself. |
@nainar adding you back since ads do not own the extensions/ folder, and we need your owner approval to create a new extension. |
@kristoferbaxter Yeah, I am. |
let src; | ||
return ( | ||
!useRemoteHtml && | ||
!!(src = element.getAttribute('src')) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is =
intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@powerivq I don 't quite understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this assignment be moved to below (line 32)? The logic here is a bit difficult to understand.
let src = element.getAttribute('src');
return (
!useRemoteHtml &&
!!src &&
....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rubber stamp LGTM
@powerivq – Looks like a |
No description provided.