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

Switch to argparse #155

Merged
merged 1 commit into from
Mar 14, 2024
Merged

Switch to argparse #155

merged 1 commit into from
Mar 14, 2024

Conversation

Ayush1325
Copy link
Contributor

@Ayush1325 Ayush1325 commented Jan 10, 2024

I have kept any changes not directly related to argparse out of this PR. However, switching to it should allow a lot more code cleaning in the future.

Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
@JelmerT
Copy link
Owner

JelmerT commented Mar 14, 2024

This one was long overdue and a great improvement, thanks so much!

@JelmerT JelmerT merged commit f42a2e2 into JelmerT:master Mar 14, 2024
@Ayush1325 Ayush1325 deleted the argparse branch March 15, 2024 05:21
@git-developer
Copy link
Contributor

The type change of port is incompatible to previous versions, see #161. What's the rationale behind this? The documentation was not updated.

@git-developer
Copy link
Contributor

After looking at the code, I get the impression that this was not intended. The following types are set:

  • len: int, had a number as default before => OK
  • baud: int, had a number as default before => OK
  • address: int, used device.flash_start_addr as default before which is a number => OK
  • port: int, used as string in the code

Please confirm whether you regard #163 as a valid fix.

@JelmerT
Copy link
Owner

JelmerT commented Mar 18, 2024

Thanks for the catch and fix @git-developer, I definitely didn't test this PR enough before it was pulled, my bad.
But at least now it's in there and being tested ;)

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