Skip to content

add lagkv_press#77

Merged
alessiodevoto merged 9 commits intoNVIDIA:mainfrom
JoelSeniorLiang:lagkv
Jun 10, 2025
Merged

add lagkv_press#77
alessiodevoto merged 9 commits intoNVIDIA:mainfrom
JoelSeniorLiang:lagkv

Conversation

@JoelSeniorLiang
Copy link
Copy Markdown
Contributor

@JoelSeniorLiang JoelSeniorLiang commented Jun 3, 2025

#75 PR description

Description of your PR. Fixes # (issue) (if applicable)

Checklist

  • Tests are working (make test)
  • Code is formatted correctly (make style, on errors try fix with make format)
  • Copyright header is included
  • All commits are signed-off using git commit -s
  • (new press) mypress_press.py is in the presses directory
  • (new press) MyPress is in __init__.py
  • (new press) README.md is updated with a 1 liner about the new press in the Available presses section
  • (new press) new press is in the default_presses list in tests/default_presses.py

Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
@SimJeg SimJeg requested a review from alessiodevoto June 5, 2025 07:11
@alessiodevoto
Copy link
Copy Markdown
Collaborator

Hi and thanks for opening this PR! 


The code looks ok, there are a few things to be adjusted before merging:

  • Could you add your press in tests/default_presses.py
  • Could you update the README with a short description of the press
  • Could you please run make style to make sure the format is ok
  • I left some comments in the review, concerning minor code changes for readability

@JoelSeniorLiang
Copy link
Copy Markdown
Contributor Author

Thanks for the comments. I will fix them.

Copy link
Copy Markdown
Collaborator

@alessiodevoto alessiodevoto left a comment

Choose a reason for hiding this comment

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

Some additional minor changes

@alessiodevoto
Copy link
Copy Markdown
Collaborator

@JoelSeniorLiang also, because we updated the main, don't forget to merge it into your branch, thanks 🙂

@JoelSeniorLiang
Copy link
Copy Markdown
Contributor Author

fixs are commit

Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
Signed-off-by: Kyle Liang <liangmanlai@gmail.com>
Copy link
Copy Markdown
Contributor Author

@JoelSeniorLiang JoelSeniorLiang left a comment

Choose a reason for hiding this comment

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

fix all the required changes.

@alessiodevoto alessiodevoto merged commit f3c7829 into NVIDIA:main Jun 10, 2025
3 checks passed
maxjeblick pushed a commit that referenced this pull request Aug 12, 2025
Signed-off-by: Max Jeblick <maximilianjeblick@gmail.com>
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