-
Notifications
You must be signed in to change notification settings - Fork 205
REFACTOR: run pyaedt script py #6970
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6970 +/- ##
==========================================
- Coverage 83.49% 82.19% -1.31%
==========================================
Files 252 252
Lines 78607 78607
==========================================
- Hits 65631 64608 -1023
- Misses 12976 13999 +1023 🚀 New features to boost your workflow:
|
|
@hui-zhou-a I think it's a good idea to include a textbox like this to allow args to be added easily. However, I don't think |
| top += 30 | ||
| # Configure file | ||
| lbl_cfg = Label() | ||
| lbl_cfg.Text = "--configure_file" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be discussed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, this is what we discussed. We can only accept "Script Arguments:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. I will create an extension to add configure_file support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @hui-zhou-a but why the configuration file is not just a script argument?
Even an extension is not a solution, we want to avoid adding an extension only for this, we will not allow to merge it either.
please explain why the script argument is not enough
Description
Please provide a brief description of the changes made in this pull request.
Issue linked
Please mention the issue number or describe the problem this pull request addresses.
Checklist