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

[Windows Installer] Prevent handle leaks #5170

Merged
merged 1 commit into from
Mar 27, 2023
Merged

[Windows Installer] Prevent handle leaks #5170

merged 1 commit into from
Mar 27, 2023

Conversation

BrianNixon
Copy link
Contributor

Fixes #4178

@talregev
Copy link
Contributor

@BrianNixon
Is it interning you implementing the new installer?
#5064

@AenBleidd
Copy link
Member

@BrianNixon

Is it interning you implementing the new installer?

#5064

Definitely not because there is no decision yet about the new installer toolkit

@talregev
Copy link
Contributor

Is it interesting you to do this research?

@BrianNixon
Copy link
Contributor Author

Is it interesting you to do this research?

At the moment I don’t know enough about installers, or BOINC’s setup needs, to be able to take on that task.

I just picked an easy ‘Newbie’ bug to fix…

@talregev
Copy link
Contributor

talregev commented Mar 27, 2023

Is it interesting you to do this research?

At the moment I don’t know enough about installers, or BOINC’s setup needs, to be able to take on that task.

I just picked an easy ‘Newbie’ bug to fix…

Let me know when you feel ready. Thank you for your fix!

The task is to learn about the installers.
Also it written on the issue a full list of boinc needs from the new installer.

@AenBleidd
Copy link
Member

Is it interesting you to do this research?

I believe this should be done by me and the decision should be made by me as well

Copy link
Member

@AenBleidd AenBleidd left a comment

Choose a reason for hiding this comment

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

Looks good to me

@talregev
Copy link
Contributor

talregev commented Mar 27, 2023

Is it interesting you to do this research?

I believe this should be done by me and the decision should be made by me as well

As you pointed out in the issue you don't have time for that. So stop blocking others for helping the boinc community.

@AenBleidd
Copy link
Member

As you pointed out in the issue you don't have time for that. So stop blocking others for helping the boinc community.

I don't stop anybody. I'm just saying that this is a complex task, and it's probably better if I do this.
If somebody else has experience - they are always welcome to contribute.

@AenBleidd AenBleidd merged commit 268cddb into BOINC:master Mar 27, 2023
@talregev
Copy link
Contributor

As you pointed out in the issue you don't have time for that. So stop blocking others for helping the boinc community.

I don't stop anybody. I'm just saying that this is a complex task, and it's probably better if I do this. If somebody else has experience - they are always welcome to contribute.

If you say it probably should done by me, it blocking people from learning and give the information for windows installer.

@AenBleidd
Copy link
Member

If you say it probably should done by me, it blocking people from learning and give the information for windows installer.

At first, this is not a correct place for such a discussion.
Second, I expressed my opinion, because this is really something that could be more effectively done by me rather than by someone who just starts to learn BOINC codebase. But I still don't block anyone from doing that.

@BrianNixon BrianNixon deleted the win_launcher_handle_leak branch March 27, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle leak in launcher code
3 participants