Skip to content
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

Put main code in a function #291

Closed
wants to merge 1 commit into from

Conversation

Timmmm
Copy link
Contributor

@Timmmm Timmmm commented Oct 4, 2024

This is a small change that has a couple of advantages:

  1. It lets other Python code import this module and run main().
  2. When this code is converted to a modern pyproject-based setup it will be required in order to expose this script.

This is a small change that has a couple of advantages:

1. It lets other Python code import this module and run `main()`.
2. When this code is converted to a modern pyproject-based setup it will be required in order to expose this script.
@aswaterman
Copy link
Member

@IIITM-Jay please review.

@@ -1046,7 +1046,7 @@ def signed(value, width):
return value - (1<<width)


if __name__ == "__main__":
def main():
Copy link
Member

@IIITM-Jay IIITM-Jay Oct 5, 2024

Choose a reason for hiding this comment

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

@aswaterman yes, we can do this change as I have already did that in Refactored parse.py file in my PR ( a longer one). So yeah, this could be accepted (but I have already done in my PR) .

Copy link
Member

Choose a reason for hiding this comment

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

Let's close this PR and handle this as part of merging your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@IIITM-Jay which PR are you referring to? I can't find one that does this.

Copy link
Member

@IIITM-Jay IIITM-Jay Oct 28, 2024

Choose a reason for hiding this comment

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

@Timmmm I have revised the PR #283 with mentioning in comments, about only to consider that PR with pure intention to refactor and optimize the parser logic with modular approach to enhance maintainability and readability keeping the best practices and standards.

Will include this thing as I proceed further refactoring other scripts which are dedicated to generate the corresponding artifacts such as encoding.out.h, instr-table.tex etc.
I have in mind to include the changes w.r.t main method inclusion.

@aswaterman aswaterman closed this Oct 7, 2024
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.

3 participants