Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use formmated custom logger, fix bug parsing env private key #72

Merged
merged 7 commits into from
Dec 10, 2023
Merged

Use formmated custom logger, fix bug parsing env private key #72

merged 7 commits into from
Dec 10, 2023

Conversation

alexstolr
Copy link

No description provided.

@alexstolr
set log level to log (above debug) unless debug explicitly stated
fd17063 
remove all nest js related prints. change all console to logger

fix merge issue
@alexstolr
add some config prints.
02ed356
@alexstolr
fix bug. App starts even when private key not provided
8a80f59 
env var was implicitly converted to the string of 'undefined'
@alexstolr
use custom logger
21efd4c
IlyaVi
IlyaVi approved these changes Dec 3, 2023
View reviewed changes
src/shared/services/conf.service.ts Outdated Show resolved Hide resolved
meshin-dev
meshin-dev suggested changes Dec 5, 2023
View reviewed changes
src/modules/clusters/cli/cluster.transformer.tsx Outdated
@@ -1,5 +1,6 @@
import TimeAgo from 'javascript-time-ago';
import en from 'javascript-time-ago/locale/en';
import {CustomLogger} from "@cli/shared/services/logger.service";
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're using webstorm, add extension to support .editorconfig and also enable eslint --fix on save.
For example, this import should have spaces before and after class name

Copy link
Author

Choose a reason for hiding this comment

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

V

src/modules/clusters/cli/cluster.transformer.tsx Outdated
@@ -80,7 +82,7 @@ export const transformClusterData = (items, extra: any) => {
return order.indexOf(a.status.text) - order.indexOf(b.status.text);
});
} catch (e) {
console.log(e);
logger.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

Add more descriptive explanation, showing that it was attempt to trnsform cluster data. not just raw error

Copy link
Author

Choose a reason for hiding this comment

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

V

src/modules/earnings/cli/earning.transformer.tsx Outdated
@@ -1,9 +1,12 @@
import {CustomLogger} from "@cli/shared/services/logger.service";
Copy link
Contributor

Choose a reason for hiding this comment

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

same in all files with imports

Copy link
Author

Choose a reason for hiding this comment

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

V

src/modules/earnings/cli/earning.transformer.tsx Outdated
@@ -16,7 +19,7 @@ export const transformEarningData = items => {
});
}
} catch (e) {
console.log(e);
logger.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

more details

Copy link
Author

Choose a reason for hiding this comment

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

V

src/services/worker/cron/fetch.cron.ts Outdated
constructor(private _fetchTask: FetchTask) {}

@Cron(CronExpression.EVERY_SECOND)
async fetchNewValidators(): Promise<void> {
try {
await this._fetchTask.fetchAllEvents();
} catch (e) {
console.log(e);
this._logger.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

same

src/services/worker/cron/liquidation.cron.ts
constructor(private _liquidationTask: LiquidationTask) {}

@Cron(CronExpression.EVERY_10_SECONDS)
async liquidate(): Promise<void> {
try {
await this._liquidationTask.liquidate();
} catch (e) {
console.log(e);
this._logger.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Author

Choose a reason for hiding this comment

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

V

src/services/worker/worker.tsx Outdated
// eslint-disable-next-line no-console
console.info(`WebApp is running on port: ${port}`);
app.useLogger(logger);
logger.log('API is active');
Copy link
Contributor

Choose a reason for hiding this comment

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

This is useless log

Copy link
Author

Choose a reason for hiding this comment

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

V

src/shared/services/web3.provider.ts Outdated
@@ -119,6 +115,8 @@ export class Web3Provider {
genesisBlock: jsonCoreData.genesisBlock,
};



Copy link
Contributor

Choose a reason for hiding this comment

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

Too many empty lines

Copy link
Author

Choose a reason for hiding this comment

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

V

src/shared/services/web3.provider.ts Outdated
@@ -131,9 +129,13 @@ export class Web3Provider {
hash: new Web3().utils.keccak256(error).substring(0, 10),
});
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Not necessary to have this empty line

Copy link
Author

@alexstolr alexstolr Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Looking at the code, the constructor is too long and should be broken down to small functions, having some empty lines seperates different common logics in the function and makes it more readable imo. I prefer not start to refactor it as part of the CR of this PR. will do it in a different one. removed the space.

src/shared/services/web3.provider.ts Outdated
@@ -149,6 +151,16 @@ export class Web3Provider {
return this.contract.abiViews as any;
}

async printConfig(){

Copy link
Contributor

Choose a reason for hiding this comment

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

Empty line

Copy link
Author

Choose a reason for hiding this comment

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

It's not there already

@alexstolr
Copy link
Author

@meshin-dev I addresses all your comments, please re-review

@alexstolr alexstolr merged commit 6a121fb into stage Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vadiminc vadiminc vadiminc approved these changes

@IlyaVi IlyaVi IlyaVi approved these changes

@meshin-dev meshin-dev meshin-dev approved these changes

@axelrod-blox axelrod-blox axelrod-blox approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@alexstolr @vadiminc @IlyaVi @meshin-dev @axelrod-blox