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

creat branch dev #1

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

creat branch dev #1

wants to merge 8 commits into from

Conversation

nekruz03
Copy link
Contributor

@nekruz03 nekruz03 commented Jan 9, 2024

No description provided.

Choose a reason for hiding this comment

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

Пожалуйста, удалите то, что не относится непосредственно к вашему проекту или замените на свою иконку

README.md Outdated
@@ -0,0 +1 @@
Для запуска - npm start

Choose a reason for hiding this comment

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

к ПР не приложена ссылка не демо, где собрана визитка

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Добавил ссылку на демо https://nekruz03.github.io/

Copy link

@stepancar stepancar left a comment

Choose a reason for hiding this comment

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

Оставил комментарии

Copy link

@stepancar stepancar left a comment

Choose a reason for hiding this comment

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

Lint не проходит, https://github.com/stepancar-web-programming/2023-fall-lab-portfolio-nazirdzhanov-n.-f./actions/runs/7515429921/job/20459338958#step:5:9

Оставил комментарий по работе для защиты
#1 (comment)

Сам сайт портфолио выглядит достойно

- name: Set up Node.js
uses: actions/setup-node@v3
with:
node-version: '14'

Choose a reason for hiding this comment

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

Устаревшая версия nodejs

Choose a reason for hiding this comment

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

@@ -0,0 +1,3 @@
/node_modules
/dist
package-lock.json
Copy link

@stepancar stepancar Jan 24, 2024

Choose a reason for hiding this comment

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

package-lock.json не должен быть игнорирован,
Нужно вспомнить про назначение этого файла.

Comment on lines +6 to +8
"@testing-library/jest-dom": "^5.11.10",
"@testing-library/react": "^11.2.5",
"@testing-library/user-event": "^12.8.3",

Choose a reason for hiding this comment

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

это devDependency

Choose a reason for hiding this comment

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

Этот файл должен быть в gitignore если вы его случайно коммитите и он не влияет на проект

Choose a reason for hiding this comment

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

В гит закоммичены и yarn.lock и package-lock.json. как потребитель поймет каким пакетным менеджером ему пользоваться в вашем проекте?

Choose a reason for hiding this comment

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

файл workflow называется lint, при этом тут происходит build. Самого запуска lint не вижу

@stepancar
Copy link

@nekruz03 , оставил комментарии, жду их решения.

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.

2 participants