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
Expanding rows #30449
Expanding rows #30449
Conversation
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
dee25ed
to
2f969c7
Compare
2f969c7
to
55cb85b
Compare
842db56
to
d63dc7c
Compare
d63dc7c
to
4877f08
Compare
} | ||
// zone.js must be loaded and processed before Angular bundle gets executed | ||
loadScript('/npm/node_modules/zone.js/dist/zone.js').then(function () { | ||
loadScript(document.location.search.endsWith('debug') ? 'bundle.min_debug.js' : 'bundle.min.js'); |
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.
This is the only reliable way of loading Angular bundle after zone.js was processed. Under Chromium version 69 used by benchpress we were randomly getting into a situation where bundle would be processed before zone.js fully loaded / executed.
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.
👍
@@ -34,7 +34,7 @@ exports.config = { | |||
framework: 'jasmine2', | |||
jasmineNodeOpts: { | |||
showColors: true, | |||
defaultTimeoutInterval: 60000, | |||
defaultTimeoutInterval: 90000, |
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.
This particular benchmark is rather big / slow and was consistently failing on my machine without this timeout increase...
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.
Consider leaving this as a comment in the file.
* tries to inject an upgraded dependency. If you see these errors, mock out the | ||
* upgraded service. | ||
*/ | ||
export const mockInjector = { |
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.
I think this can be rmoved as we are not using angular.js here
} | ||
|
||
// Waits for Angular's testability.whenStable() | ||
export async function waitForStable() { |
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.
Actually I think most of these functions can be removed
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.
You are right @mhevery - the whole file can be deleted :-) Done in a fixup commit, thnx!
} | ||
// zone.js must be loaded and processed before Angular bundle gets executed | ||
loadScript('/npm/node_modules/zone.js/dist/zone.js').then(function () { | ||
loadScript(document.location.search.endsWith('debug') ? 'bundle.min_debug.js' : 'bundle.min.js'); |
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.
👍
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
… in ViewEngine and Ivy
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
merge assistance: global approval |
…wEngine and Ivy (angular#30449) PR Close angular#30449
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixup of #30403 started by @mhevery