Skip to content

feat: add InterfaceDefStmt #269

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: add InterfaceDefStmt #269

wants to merge 3 commits into from

Conversation

Sarfia-786
Copy link
Contributor

Pull Request description

This PR added InterfaceDefStmt

Pull Request checklists

Note:

This PR is a:

  • bug-fix
  • new feature
  • maintenance

About this PR:

  • it includes tests.
  • the tests are executed on CI.
  • the tests generate log file(s) (path).
  • pre-commit hooks were executed locally.
  • this PR requires a project documentation update.

Author's checklist:

  • I have reviewed the changes and it contains no misspelling.
  • The code is well commented, especially in the parts that contain more
    complexity.
  • New and old tests passed locally.

Additional information

Reviewer's checklist

Copy and paste this template for your review's note:

## Reviewer's Checklist

- [ ] I managed to reproduce the problem locally from the `main` branch
- [ ] I managed to test the new changes locally
- [ ] I confirm that the issues mentioned were fixed/resolved .

xmnlab
xmnlab requested changes Apr 17, 2025
View reviewed changes
Copy link
Contributor

@xmnlab xmnlab left a comment

Choose a reason for hiding this comment

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

hi @Sarfia-786 ! thanks for working on that.

in the transpiler visit method you created, the types should be dynamic, you shouldn't check each one.

also avoid using long nested structures. I recommend this video that explains a bit about that: https://www.youtube.com/watch?v=CFRhGnuXG-4

@Sarfia-786 Sarfia-786 requested a review from xmnlab April 19, 2025 17:29
@Sarfia-786
Copy link
Contributor Author

hi @Sarfia-786 ! thanks for working on that.

in the transpiler visit method you created, the types should be dynamic, you shouldn't check each one.

also avoid using long nested structures. I recommend this video that explains a bit about that: https://www.youtube.com/watch?v=CFRhGnuXG-4

@xmnlab done.

@github-actions GitHub Actions
Copy link

This pull request has been marked as stale because it has been
inactive for more than 5 days. Please update this pull request
or it will be automatically closed in 5 days.

@github-actions github-actions bot added the Stale label Apr 25, 2025
xmnlab
xmnlab reviewed Apr 29, 2025
View reviewed changes
libs/astx-transpilers/src/astx_transpilers/python_string.py
"""Handle InterfaceDefStmt nodes using dynamic type visiting."""
class_header = f"class {node.name}(ABC):"
self.indent_level += 1
indent = self.indent_str * self.indent_level
Copy link
Contributor

Choose a reason for hiding this comment

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

usually we use _generate_block for handling indentation automatically. would it be possible to use that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we can use @xmnlab, all the PRs were closed by GitHub Actions. Could you please reopen them?

@github-actions github-actions bot removed the Stale label Apr 30, 2025
@github-actions GitHub Actions
Copy link

github-actions bot commented May 6, 2025

This pull request has been marked as stale because it has been
inactive for more than 5 days. Please update this pull request
or it will be automatically closed in 5 days.

@github-actions github-actions bot added the Stale label May 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xmnlab xmnlab xmnlab requested changes

Assignees
No one assigned
Labels
Stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Sarfia-786 @xmnlab