Skip to content

refactor: to biolerplate v2 (MAPCO 6292) - #53

Open
Powfu2 wants to merge 13 commits into
masterfrom
update-to-biolerplate-v2
Open

refactor: to biolerplate v2 (MAPCO 6292)#53
Powfu2 wants to merge 13 commits into
masterfrom
update-to-biolerplate-v2

Conversation

@Powfu2

Copy link
Copy Markdown
QuestionAnswer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Related issues: MAPCO-6292
Closes #XXX ...

Further information:

@Powfu2
Powfu2 requested a review from NivGreensteinJuly 9, 2026 15:52
Comment thread.github/workflows/pull_request.yaml Outdated
Comment on lines 67 to 69
POSTGRES_PASSWORD: postgres
POSTGRES_USER: postgres
POSTGRES_DB: postgres

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note:
You can add top-level ENVs and reference them.
See example here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove the env
you added a top-level env

Comment threadconfig/default.json
Comment on lines +44 to +45
"database": "host",
"schema": "postgres"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

revert please

Suggested change
"database": "host",
"schema": "postgres"
"database": "dump-server",
"schema": "dump_server"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

this is already reverted dunno why its still here

Comment threadsrc/common/db/index.ts Outdated
...connectionOptions,
password: undefined,
ssl: {
key: readFileSync(ssl.key),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If readFilesSync fails, it throws an error. make sure it throws an indicative one

Comment threadsrc/common/db/index.ts Outdated
const dbConfig= config.get<DbConfig>('db');
const connectionOptions = createConnectionOptions({ entities: ENTITIES_DIRS, ...dbConfig });
const config = container.resolve<ConfigType>(SERVICES.CONFIG);
const dbConfig: DbConfig = config.get('db');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need to assign DbConfig type. The type will auto resolve

Comment threadsrc/common/constants.ts Outdated
import { readPackageJsonSync } from '@map-colonies/read-pkg';

export const SERVICE_NAME = readPackageJsonSync().name ?? 'unknown_service';
export const DEFAULT_SERVER_PORT = 80;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please default to port 8080

Comment threadsrc/containerConfig.ts Outdated
Comment on lines +107 to +109
useFactory: (container): HealthCheck => {
const connection = container.resolve<Connection>(Connection);
return getDbHealthCheckFunction(connection);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

create a function somewhere else. Don't create logic at containerConfig

Comment threadsrc/index.ts Outdated
Comment on lines +23 to +24
console.error('😢 - failed initializing the server');
console.error(error);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not using logger if its available? revert relevant changes

Comment threadsrc/instrumentation.mts
Comment threadtests/helpers/index.ts
Comment threadormconfig.ts Outdated
Comment on lines +5 to +6
// The typeorm CLI cannot await the async application config, so the db section is read directly
// from the local config file, honoring the same DB_* env overrides the schema defines for the application.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ai slop?

@NivGreenstein

Copy link
Copy Markdown
Contributor

Also, missing some files

Comment thread.github/workflows/pull_request.yaml Outdated
DB_NAME: postgres
DB_USERNAME: postgres
DB_PASSWORD: postgres
# DB_SCHEMA: public

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove comments

Comment thread.github/workflows/pull_request.yaml Outdated
Comment on lines 67 to 69
POSTGRES_PASSWORD: postgres
POSTGRES_USER: postgres
POSTGRES_DB: postgres

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove the env
you added a top-level env

Comment threadsrc/common/constants.ts Outdated

export const SERVICE_NAME = readPackageJsonSync().name ?? 'unknown_service';
export const DEFAULT_SERVER_PORT = 80;
export const DB_HEALTHCHECK_TIMEOUT_MS = 5000;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove it if you added it to the config

Comment threadsrc/containerConfig.ts Outdated
Comment threadsrc/containerConfig.ts Outdated
Comment threadsrc/containerConfig.ts Outdated
Comment threadormconfig.ts Outdated
Comment threadsrc/common/db/index.ts
Comment threadpackage.json
"@map-colonies/openapi-express-viewer": "^5.0.0",
"@map-colonies/prometheus": "^1.0.0",
"@map-colonies/read-pkg": "^2.0.0",
"@map-colonies/schemas": "https://ghatmpstorage.blob.core.windows.net/npm-packages/schemas-d8f2e93dca95f6e278321177197760cf47241d5b.tgz",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

make sure to change before you close the pr

@Powfu2Powfu2 changed the title Update to biolerplate v2refactor: to biolerplate v2 (MAPCO 6292)Jul 27, 2026
Sign up for freeto 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.

2 participants

@Powfu2@NivGreenstein