Skip to content

feat: tox implementation#1163

Open
moe-ad wants to merge 19 commits intomainfrom
feat/tox-implementation
Open

feat: tox implementation#1163
moe-ad wants to merge 19 commits intomainfrom
feat/tox-implementation

Conversation

@moe-ad
Copy link
Contributor

@moe-ad moe-ad commented Feb 19, 2026

Closes #815. Check the issue for context that might be missing here.

Important things to note

  • Prek is used for running the pre-commit checks
  • For updating the action reference, I opted for a python script.
  • An additional environment for running python-utils/test_versions.py was also included.

@moe-ad moe-ad self-assigned this Feb 19, 2026
@moe-ad moe-ad changed the title Feat/tox implementation feat: tox implementation Feb 19, 2026
@moe-ad moe-ad force-pushed the feat/tox-implementation branch from aa6ce23 to 2ac902a Compare February 19, 2026 10:46
@github-actions github-actions bot added the enhancement General improvements to existing features label Feb 19, 2026
@moe-ad moe-ad marked this pull request as ready for review February 19, 2026 16:03
@moe-ad moe-ad requested a review from a team as a code owner February 19, 2026 16:03
Copy link
Contributor

@SMoraisAnsys SMoraisAnsys left a comment

Choose a reason for hiding this comment

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

That would be very valuable @moe-ad, thanks for this !
I added some minor comments.

Copy link
Member

@jorgepiloto jorgepiloto left a comment

Choose a reason for hiding this comment

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

Awesome and needed pull. Just left some requests on my side.


new_content, count = re.subn(old_pattern, new_replacement, content)

if count > 0:
Copy link
Contributor

@MaxJPRey MaxJPRey Feb 23, 2026

Choose a reason for hiding this comment

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

Maybe we could write a message if count == 0 to understand the absence of update.

Only in the case of the dry-run to not be too noisy.
Not mandatory as it could become cumbersome, but could help to debug and test.

Copy link
Contributor Author

@moe-ad moe-ad Feb 24, 2026

Choose a reason for hiding this comment

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

I have added more debugging related code in 57f1327. Let me know what you think.

@moe-ad
Copy link
Contributor Author

moe-ad commented Feb 24, 2026

@jorgepiloto @SMoraisAnsys @MaxJPRey thanks for your reviews. Can you give this a look again?

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

Labels

enhancement General improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Tox

5 participants