Skip to content

Regex package #139

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

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

Regex package #139

wants to merge 17 commits into from

Conversation

msaelices
Copy link
Contributor

Checklist

  • My recipe.yaml file specifies which version(s) of MAX is compatible with my project (see here for an example). If not, my package is compatible with both 24.5 and 24.6.
  • License file is packaged (see here for an example).
  • Set the build number to 0 (for new packages, or if the version changed).
  • Bumped the build number (if the version is unchanged).

@yetalit
Copy link
Collaborator

yetalit commented Jun 30, 2025

Hi @msaelices , very cool project! But, there are several issues I can observe:

  1. You haven't included image.jpeg of your project.
  2. You have pinned your project for max >= 25.4.0 . This will work fine as 25.4.0 is currently the latest version; But, when we have max 25.5.0, your project may not be compiled successfully by the newer compiler.
  3. If I remember correctly, pixi run mojo package regex -o ${PREFIX}/lib/mojo/regex.mojopkg is not valid and causes the process to fail. We use mojo commands without pixi run as the pixi shell is already activated.
  4. Your above command also references to regex folder which is not correct. The regex folder in your repo is located at src/regex.
  5. You haven't specified the snapshot commit hash value; Example:
source:
  - git: https://github.com/msaelices/mojo-websockets.git
    rev: 751c48ed576fe5a6113afd8ae83f6e48dedf832b
  1. I see the python code in your repo is not involved in the package itself; But, I think you're still required to activate CodeQL for your repo and include its badge in the ReadMe.

Signed-off-by: Manuel Saelices <[email protected]>
Signed-off-by: Manuel Saelices <[email protected]>
@msaelices
Copy link
Contributor Author

Hi @msaelices , very cool project! But, there are several issues I can observe:

  1. You haven't included image.jpeg of your project.
  2. You have pinned your project for max >= 25.4.0 . This will work fine as 25.4.0 is currently the latest version; But, when we have max 25.5.0, your project may not be compiled successfully by the newer compiler.
  3. If I remember correctly, pixi run mojo package regex -o ${PREFIX}/lib/mojo/regex.mojopkg is not valid and causes the process to fail. We use mojo commands without pixi run as the pixi shell is already activated.
  4. Your above command also references to regex folder which is not correct. The regex folder in your repo is located at src/regex.
  5. You haven't specified the snapshot commit hash value; Example:
source:
  - git: https://github.com/msaelices/mojo-websockets.git
    rev: 751c48ed576fe5a6113afd8ae83f6e48dedf832b
  1. I see the python code in your repo is not involved in the package itself; But, I think you're still required to activate CodeQL for your repo and include its badge in the ReadMe.

I think I've fixed every missing point

Signed-off-by: Manuel Saelices <[email protected]>
Signed-off-by: Manuel Saelices <[email protected]>
Signed-off-by: Manuel Saelices <[email protected]>
@carolinefrasca
Copy link
Collaborator

@msaelices do you mind enabling CodeQL scanning on the source repo and adding this badge to the README?

![CodeQL](https://github.com/{username}/{repo name}/workflows/CodeQL/badge.svg)

@msaelices
Copy link
Contributor Author

![CodeQL](https://github.com/{username}/{repo name}/workflows/CodeQL/badge.svg)

The CodeQL was already enabled. I've just added the badge. Thanks!

msaelices and others added 5 commits July 1, 2025 10:26
@yetalit
Copy link
Collaborator

yetalit commented Jul 2, 2025

Hi @msaelices , very cool project! But, there are several issues I can observe:

  1. You haven't included image.jpeg of your project.
  2. You have pinned your project for max >= 25.4.0 . This will work fine as 25.4.0 is currently the latest version; But, when we have max 25.5.0, your project may not be compiled successfully by the newer compiler.
  3. If I remember correctly, pixi run mojo package regex -o ${PREFIX}/lib/mojo/regex.mojopkg is not valid and causes the process to fail. We use mojo commands without pixi run as the pixi shell is already activated.
  4. Your above command also references to regex folder which is not correct. The regex folder in your repo is located at src/regex.
  5. You haven't specified the snapshot commit hash value; Example:
source:
  - git: https://github.com/msaelices/mojo-websockets.git
    rev: 751c48ed576fe5a6113afd8ae83f6e48dedf832b
  1. I see the python code in your repo is not involved in the package itself; But, I think you're still required to activate CodeQL for your repo and include its badge in the ReadMe.

I think I've fixed every missing point

Apparently, 2nd and 3rd issues are still missing. If you are not convinced with 2nd issue; When max 25.5.0 comes out, to merge new packages, all previous packages will also be compiled. As you specified your package to be compatible with ANY VERSION above 25.4.0, it will try to compile your package with max 25.5.0; And this will most likely fail and prevent us from adding new submitted packages.

@yetalit
Copy link
Collaborator

yetalit commented Jul 2, 2025

@msaelices FYI, the way you defined requirements part was also causing the process to fail. You may want to modify your recipe generator script accordingly.

@yetalit yetalit self-requested a review July 2, 2025 08:43
@yetalit yetalit requested review from yetalit and removed request for yetalit July 2, 2025 08:44
@msaelices
Copy link
Contributor Author

@yetalit thanks for your fix! I was doing the same and then I realized you did it for me. Thanks so much!
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants