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.

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

Sprint 2 #6

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

Sprint 2 #6

wants to merge 34 commits into from

Conversation

zloegucci
Copy link
Owner

No description provided.

Copy link

@dmitrymorozoff dmitrymorozoff left a comment

Choose a reason for hiding this comment

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

Привет. Мне понравилась твоя работу. В целом по ТЗ все выполнено. Несколько комментариев, где что-то можно улучшить а в остальном все ок. Работу принял

"devDependencies": {
"@parcel/transformer-pug": "2.7.0",
"@parcel/transformer-sass": "2.7.0",
"@types/node": "^18.7.19",

Choose a reason for hiding this comment

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

Нужно зафиксировать зависимости

regNickname,
regName,
regSurname,
regTelephone,

Choose a reason for hiding this comment

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

Чтобы не перечислять все поля, можно было бы воспользоваться деструктуризацией

import ProxyProps from './ProxyProps';

interface IChildren {
[key: string]: Block;

Choose a reason for hiding this comment

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

Для такой записи в typescript есть специальный тип Record

FLOW_RENDER = 'flow:render',
}

export class Block {

Choose a reason for hiding this comment

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

А почему здесь не указан дженерик? Block<TProps extends {}>

eventBus.emit(Events.INIT, {});
}

_registerEvents(eventBus: EventBus) {

Choose a reason for hiding this comment

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

Приватные методы нужно писать с модификатором доступа private, иначе по умолчанию все методы public

@@ -0,0 +1,29 @@
export default function isValidName(value: string) {

Choose a reason for hiding this comment

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

Во всех функциях стоит прописывать тип возвращаемого значения

if (/[^a-zA-Zа-яА-Я\-]+/.test(value)) {
return { error: true, errorText: "должны быть только буквы" };
}
const ans = regExp.exec(value);

Choose a reason for hiding this comment

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

Советую не "экономить на спичках" и не сокращать названия переменных. Выигрыш от этого в несколько символов, а при чтении вообще не понятно, что есть что. Следует во всем проекте не использовать сокращения, пусть код будет понятный и читаемый

errorText: "первая буква должна быть заглавной, остальное - нет",
};
}
if (ans[0] !== value) {

Choose a reason for hiding this comment

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

Использование магических чисел в программирование это плохая практика. Статья на эту тему. В данном случае, следует создать переменную или константу, положить в нее значение и задать ей понятное название.

"strictNullChecks": true,
// "strictFunctionTypes": true, /* When assigning functions, check to ensure parameters and the return values are subtype-compatible. */
// "strictBindCallApply": true, /* Check that the arguments for 'bind', 'call', and 'apply' methods match the original function. */
// "strictPropertyInitialization": true, /* Check for class properties that are declared but not set in the constructor. */

Choose a reason for hiding this comment

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

А зачем все эти закомментированные настройки? Лучше все это удалить, чтобы не создавать беспорядок

"start:parcel": "parcel ./src/pages/index.pug --port 3000",
"build": "parcel build ./src/pages/index.pug",
"start": "npm run build && node ./server.js",
"test": "parcel ./src/pages/index.pug --port 3000"

Choose a reason for hiding this comment

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

Этот скрипт обычно используется для запуска тестов

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.

None yet

2 participants