Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
fix: unecessary program updates by removing timeout methods#1693
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Thanks for the PR,@sheetalkamat! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day. |
bradzacher commentedMar 7, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for looking into the performance for us! I really appreciate the help 😄 It looks like this change breaks the persistent parse tests, which ensure that the IDE use case is working as expected. you can run them locally from within the
|
Will do |
codecovbot commentedMar 9, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## master #1693 +/- ##==========================================- Coverage 95.21% 95.19% -0.02%========================================== Files 148 148 Lines 6953 6967 +14 Branches 2006 2007 +1 ==========================================+ Hits 6620 6632 +12- Misses 123 124 +1- Partials 210 211 +1
|
Uh oh!
There was an error while loading.Please reload this page.
@uniqueiniquity This PR is ready to review and merge with the changes we talked about |
And I am not sure failing codecov/patch is about |
@bradzacher any thoughts on the code coverage issue? it's a preemptive change for 3.9 so it makes sense that the path isn't taken. |
bradzacher commentedMar 11, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If you want to satisfy the branch coverage, you should be able to use istanbul ignore comments to stop the coverage reporting the missed branch. /* istanbul ignore if */if(isRunningNoTimeoutFix){// ....}/* istanbul ignore else */if(!isRunningNoTimeoutFix){// ....} I keep it around to try and prompt contributors to cover their code appropriately, though I don't usually treat code coverage as a hard merge blocker because there's always some edge cases and checks to keep the types "safe" (unless of course the coverage is terrible, then I RC 😅). When it fails, I usually review thecoverage report to see what code was missed, and if it's something that should be tested. If you're happy with this change, approve it and I'll gladly override the failed check to merge it. (again, thanks to you both for looking into all of this for us) |
I think we should ignore the coverage result instead of adding ignore comment since as soon as 3.9 releases things should start working... I will update this with master for merge |
This should fix unnecessary program updates that happen right away whenever something changes