Skip to content

Conversation

@NatalieShaked
Copy link

@NatalieShaked NatalieShaked commented May 6, 2025

Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Related issues: MAPCO-6131
Closes: MAPCO-6131

Close this issue before completing the PR.

@NatalieShaked NatalieShaked self-assigned this May 6, 2025
@NivGreenstein
Copy link

NivGreenstein commented May 12, 2025

Update build-and-push github workflow
add release-please workflow

import { tracingFactory } from './common/tracing.js';
import { getConfig, initConfig } from './common/config.js';

await initConfig(true);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove true arg from initConfig.
You'll need to work with config server.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the schemas PR first needs to be merged first in order for this to work. No?

Comment on lines +1 to +5
# This file instructs Redocly's linter to ignore the rules contained for specific parts of your API.
# See https://redocly.com/docs/cli/ for more information.
packages/backend/openapi3.yaml:
boolean-parameter-prefixes:
- '#/paths/~1cooldown/get/parameters/4/name'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are there 2 .redocly.lint-ignore.yaml files, one of them is redundant.

"@map-colonies/js-logger": "^2.0.0",
"@map-colonies/openapi-express-viewer": "^4.0.0",
"@map-colonies/read-pkg": "^1.0.0",
"@map-colonies/schemas": "https://ghatmpstorage.blob.core.windows.net/npm-packages/schemas-0c282bb6b722a00eb82e7791f94261b29a0d8d48.tgz",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this version is temporary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, waiting for it to be merged

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if and how the release-please pipeline works in a monorepo, this should be validated with infra team

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend @map-colonies/tsconfig/tsconfig-app and keep necessary changes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend @map-colonies/tsconfig/tsconfig-app and keep necessary changes

Comment on lines 34 to 37
"eslint": "^8.56.0",
"jest": "^29.5.0",
"jest-create-mock-instance": "^2.0.0",
"jest-html-reporters": "^3.1.4",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update eslint and jest deps, this is also true for all other monorepo packages.

Comment on lines 34 to 37
"http-status-codes": "^2.2.0",
"qs": "^6.11.2"
},
"devDependencies": {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure all environmental packages are upgraded - typescript, jest, etc.


beforeAll(() => {
mockedRedis = createClient({}) as jest.Mocked<RedisClient>;
mockedRedis = redisMock.mockRedisClient as unknown as jest.Mocked<RedisClient>;
Copy link
Collaborator

@melancholiai melancholiai Jul 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

declare the imported mock as jest.Mocked<RedisClient> in the mocks dir instead of casting here, do this for all 3 mocks

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is problematic. Instead of configuring the whole package with ignores - locate where ignores are absolutely required and only ignore them there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants