-
Notifications
You must be signed in to change notification settings - Fork 105
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
Fix run with progress and add new Run function #69
Fix run with progress and add new Run function #69
Conversation
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.
Hi @elivlo ! Thanks for your continued interest in the project :) I haven't had much time to work on it these days so I really appreciate your help. Would you like to become maintainer of the project?
I have some minor suggestions for this specific PR but the idea seems good to me. I did not test it yet though.
Hi, let me say it is an honour for me to have the opportunity to become a maintainer! I would really appreciate it! But I also cannot say how much time I'm able to spend for maintaining. I will try my best. |
@elivlo I've sent you an invite, and no worries, no strings attached :) This way though, you can even create the tag directly once this gets merged 👍 |
+ Also add simple Unit Test
@Ullaakut Hi, I have resolved your suggestions.
|
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.
A few more minor suggestions, but for me the test is fine as is even if we're not at 100% coverage :) Good job!
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.
LGTM, thanks for your contribution :)
Hi,
I fixed the function RunWithProgress because there was a memory leak (line 212) when cancelling the context before the scan was finished.
I also added a new "Run" type of funtion (RunWithStreamer) that writes the xml-output directly to a file.
There is also an example showing how to run this type of scan.
Best regards!
P.S.
I would also appreciate a new tag :-)