-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
fix(core): Change VariablesService to DI and use caching #6827
fix(core): Change VariablesService to DI and use caching #6827
Conversation
…or-injection # Conflicts: # packages/cli/src/AbstractServer.ts # packages/cli/src/Queue.ts
…or-injection # Conflicts: # packages/cli/package.json # pnpm-lock.yaml
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #6827 +/- ##
=======================================
Coverage 24.76% 24.76%
=======================================
Files 3131 3131
Lines 190781 190809 +28
Branches 21011 21012 +1
=======================================
+ Hits 47239 47261 +22
- Misses 142588 142592 +4
- Partials 954 956 +2
☔ View full report in Codecov by Sentry. |
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.
Other than the one issue it looks good to me.
… into pay-644-variables-loading-might-be-slow
Great PR! Please pay attention to the following items before merging: Files matching
Files matching
Make sure to check off this list before asking for review. |
packages/cli/src/environments/sourceControl/sourceControlImport.service.ee.ts
Outdated
Show resolved
Hide resolved
|
1 flaky tests on run #1753 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
Inline expression editor > should resolve $parameter[] |
Output
Screenshots
Video
|
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.
✅ All Cypress E2E specs passed |
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.
Approving but ideally let's fix the test in a followup 🙏🏻
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.
ActiveWorkflowRunner
tests are also failing
✅ All Cypress E2E specs passed |
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.
Approving but let's wait for the tests!
✅ All Cypress E2E specs passed |
Got released with |
No description provided.