Skip to content

Mohammed A - #12

Open
mohammedalfakih-dev wants to merge 1 commit into
HackYourAssignment:mainfrom
mohammedalfakih-dev:main
Open

mohammedalfakih-dev wants to merge 1 commit into
HackYourAssignment:mainfrom
mohammedalfakih-dev:main

Conversation

@mohammedalfakih-dev

Copy link
Copy Markdown

No description provided.

@github-actions

github-actions Bot commented Apr 8, 2026

Copy link
Copy Markdown

📝 HackYourFuture auto grade

Assignment Score: 0 / 100 ✅

Status: ✅ Passed
Minimum score to pass: 0
🧪 The auto grade is experimental and still being improved

Test Details

@rafaelhdr rafaelhdr left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Congrats. Just minor comments, but all good.

Comment thread .github/workflows/ci.yml
branches:
- main
- master
pull_request:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice that you have main and master for push, but shouldn't you also limit in the pull request? To not build on every PR?

Comment thread .github/workflows/ci.yml

defaults:
run:
working-directory: task-1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is good approach. Make easier than repeat on every step.

Comment thread .github/workflows/ci.yml
cache-dependency-path: task-1/package-lock.json

- name: Install dependencies
run: npm install

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In a github action, the ideal is to use npm ci instead of the npm install

Comment thread .github/workflows/ci.yml
run: npm run lint

- name: Run unit tests
run: npm test -- --run

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why the -- --run instead of only the npm test?

@rafaelhdr rafaelhdr self-assigned this Apr 16, 2026
@rafaelhdr rafaelhdr added the Reviewed This assignment has been reivewed by a mentor and a feedback has been provided label Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Reviewed This assignment has been reivewed by a mentor and a feedback has been provided

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants