Skip to content

✨ Adding automatic testing and improving existing workflows#54

Open
IronMax03 wants to merge 37 commits intoAlgorithmsAcademy:mainfrom
IronMax03:main
Open

✨ Adding automatic testing and improving existing workflows#54
IronMax03 wants to merge 37 commits intoAlgorithmsAcademy:mainfrom
IronMax03:main

Conversation

@IronMax03
Copy link
Member

No description provided.

@IronMax03 IronMax03 requested a review from EonP May 3, 2025 11:27
Comment on lines 27 to 31

const algoName = match[1].trim();
const issueNumber = context.issue.number;
const newTitle = `✨ Implement ${algoName}`;

Copy link
Member

Choose a reason for hiding this comment

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

          const formattedAlgoName = algoName
            .toLowerCase()
            .replace(/\s+/g, '_')       // Replace spaces with underscores
            .replace(/[^\w_]/g, '')     // Remove special characters
            .replace(/_+/g, '_');       // Replace multiple underscores with single


          const newTitle = `✨ Implement ${formattedAlgoName}`;

          ```

issue_number: issueNumber,
title: newTitle
});

Copy link
Member

Choose a reason for hiding this comment

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

          await github.rest.issues.createComment({
            ...context.repo,
            issue_number: issueNumber,
            body: `📝 Algorithm has been formatted for consistency as \`${formattedAlgoName}\`. Please use this format in your implementation file names.`
          });

Copy link
Member Author

Choose a reason for hiding this comment

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

why did you remove title: newTitle?

Copy link
Member Author

Choose a reason for hiding this comment

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

body: `📝 Algorithm has been formatted for consistency as \`${formattedAlgoName}\`. Please use this format in your implementation file names. thats nice.

Copy link
Member

Choose a reason for hiding this comment

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

why did you remove title: newTitle?
Did not remove it I suggested adding a comment

Comment on lines +1 to +4
numpy
ruff
rich
pytest
Copy link
Member

Choose a reason for hiding this comment

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

Delete this file, I do no like this

Copy link
Member Author

Choose a reason for hiding this comment

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

This approach is more scalable.

Copy link
Member

Choose a reason for hiding this comment

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

No this is absolutely not scalable, uv does that for us

Comment on lines 15 to 18
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.12'
Copy link
Member

Choose a reason for hiding this comment

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

      - name: "Set up Python"
        uses: actions/setup-python@v5
        with:
          python-version-file: ".python-version"

Copy link
Member Author

Choose a reason for hiding this comment

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

By default the action /setup-python@v4 use ".python-version", if ".python-version" cant be found then "python-version: '3.12'" is used. But if you want to remove it simply remove "with" as below.

      - name: "Set up Python"
        uses: actions/setup-python@v5

setup-python Documentation

Copy link
Member

Choose a reason for hiding this comment

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

Yes so let's remove it like you suggested


steps:
- name: Download Repo In Docker
uses: actions/checkout@v3
Copy link
Member

Choose a reason for hiding this comment

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

checkout@v4

uses: actions/checkout@v3
with:
fetch-depth: 0

Copy link
Member

@eliemada eliemada May 3, 2025

Choose a reason for hiding this comment

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

  - name: Install uv
    uses: astral-sh/setup-uv@v5
    with:
      enable-cache: true
      cache-dependency-glob: "uv.lock"

  - name: Install the project
    run: uv sync --locked --all-extras --dev

Copy link
Member Author

Choose a reason for hiding this comment

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

uv is already installed by:

    - name: Install dependencies
      run: pip install -r requirements.txt

Copy link
Member

Choose a reason for hiding this comment

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

Yes but requirements.txt is a terrible design choice

Copy link
Member Author

@IronMax03 IronMax03 May 3, 2025

Choose a reason for hiding this comment

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

Is this line complementary or does it replace the code below?

uses: actions/checkout@v3
      with:
        fetch-depth: 0

Comment on lines 20 to 21
- name: Install dependencies
run: pip install -r requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

Those can be deleted

Copy link
Member Author

Choose a reason for hiding this comment

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

No, pytest as to be installed in order to run testcases.

Copy link
Member

Choose a reason for hiding this comment

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

pytest is installed inside uv so we can delete this

run: pip install -r requirements.txt

- name: Run tests
run: pytest tests --collect-only
Copy link
Member

Choose a reason for hiding this comment

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

uv run pytest tests

uses: actions/checkout@v3
with:
fetch-depth: 0

Copy link
Member

Choose a reason for hiding this comment

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

Yes but requirements.txt is a terrible design choice

Comment on lines 15 to 18
- name: Set up Python
uses: actions/setup-python@v4
with:
python-version: '3.12'
Copy link
Member

Choose a reason for hiding this comment

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

Yes so let's remove it like you suggested

Comment on lines 20 to 21
- name: Install dependencies
run: pip install -r requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

pytest is installed inside uv so we can delete this

issue_number: issueNumber,
title: newTitle
});

Copy link
Member

Choose a reason for hiding this comment

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

why did you remove title: newTitle?
Did not remove it I suggested adding a comment

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