feat: infrastructure and boilerplate deps upgrade (MAPCO-9937)#82
feat: infrastructure and boilerplate deps upgrade (MAPCO-9937)#82roicohen326 wants to merge 9 commits intomasterfrom
Conversation
CL-SHLOMIKONCHA
left a comment
CL-SHLOMIKONCHA
left a comment
There was a problem hiding this comment.
Nice job, focus on:
- merge similar imports
- IConfig should be removed and replaced by :
ConfigType - Also remove
tsconfig.lint.json&tsconfig.test.json - Eslint error
- Failed tests
- Some missing config-management envs
| {{/* | ||
| Returns the metrics url from global if exists or from the chart's values | ||
| */}} | ||
| {{- define "ingestion-trigger.metricsUrl" -}} |
There was a problem hiding this comment.
metrics url is redundant
| {{/* | ||
| Returns the metrics buckets from global if exists or from the chart's values | ||
| */}} | ||
| {{- define "ingestion-trigger.metricsBuckets" -}} |
There was a problem hiding this comment.
metrics buckets is redundant
There was a problem hiding this comment.
would prefer to remain work with the common.tracing.merged within the _tplValues.tpl file
| {{ if $metrics.enabled }} | ||
| TELEMETRY_METRICS_ENABLED: {{ $metrics.enabled | quote }} | ||
| TELEMETRY_METRICS_URL: {{ $metrics.url }} | ||
| TELEMETRY_METRICS_ENABLED: {{ $metricsEnabled | quote }} |
There was a problem hiding this comment.
matrics envs redundant
There was a problem hiding this comment.
missing configManagement envs
| openAsync: jest.fn().mockImplementation(actualModule['openAsync'] as (...args: unknown[]) => unknown), | ||
| }; | ||
| }); | ||
| import httpStatusCodes from 'http-status-codes'; |
There was a problem hiding this comment.
handle those eslint errors
| }); | ||
|
|
||
| beforeEach(() => { | ||
| registerDefaultConfig(); |
There was a problem hiding this comment.
please recheck this one, should not be called after the getApp?
| import { trace } from '@opentelemetry/api'; | ||
| import * as gdal from 'gdal-async'; | ||
|
|
||
| jest.mock('gdal-async', () => { |
There was a problem hiding this comment.
usually jest.mock must be shown above all import, please recheck it.
| bundledApi.yaml | ||
|
|
||
| # Temporary schema files | ||
| Schema/ |
There was a problem hiding this comment.
You are right, this was added during cleanup of local generated files and is not required for this PR, so I will remove it.
There was a problem hiding this comment.
should see why you had to use taskId with !
There was a problem hiding this comment.
After the ESLint v9 migration, stricter rules exposed older tolerated patterns, so I fixed the real code issues first and used a few tactical suppressions only where external package conventions didn’t align with lint rules.
These suppressions are short term safeguards for migration stability, and I will remove them as part of targeted cleanup refactors in the upcoming PR.
3 participants
Related issues: #XXX , #XXX ...
Closes #XXX ...
Further information: