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

feat: replace unmaintained winapi with windows-sys #18

Merged
merged 4 commits into from
Jul 4, 2023

Conversation

panekj
Copy link
Contributor

@panekj panekj commented Oct 23, 2022

winapi has been unmaintained for a long time
this commit replaces winapi with windows-sys which is generated from windows API headers and maintained by Microsoft
apart from that, codebase has been fixed from few clippy lints and errors

winapi is still an indirect dependency via:

winapi has been unmaintained for a long time
this commit replaces winapi with windows-sys which is generated from
windows API headers and maintained by Microsoft
@panekj
Copy link
Contributor Author

panekj commented Oct 23, 2022

@joaomoreno what's the proper way to test all functionality?
I'll admit I'm not an expert in writing safe code
I've been able to check only --gui parameter
image

@panekj
Copy link
Contributor Author

panekj commented Oct 23, 2022

I've also tried to maintain preexisting style of codebase and added rustfmt config that preserves it

@panekj
Copy link
Contributor Author

panekj commented Oct 23, 2022

I'm also not sure what Rust version you use (CI seems to use stable) so I've updated code to latest rust compatibility (1.64, it will fail for anything older)

@joaomoreno
Copy link
Member

I've also tried to maintain preexisting style of codebase and added rustfmt config that preserves it

Feel free to submit a different PR which brings in rustfmt with more modern rules.

@joaomoreno
Copy link
Member

@joaomoreno what's the proper way to test all functionality? I'll admit I'm not an expert in writing safe code I've been able to check only --gui parameter image

The win32 specific part is purely related to GUI, so manual testing with --gui is good enough.

@panekj panekj marked this pull request as ready for review October 25, 2022 16:37
@panekj
Copy link
Contributor Author

panekj commented Oct 25, 2022

I've also tried to maintain preexisting style of codebase and added rustfmt config that preserves it

Feel free to submit a different PR which brings in rustfmt with more modern rules.

I wouldn't say that there are any modern rules 😅 it's just what rustfmt does as default (using spaces instead of tabs)

@joaomoreno joaomoreno self-assigned this Jul 4, 2023
@joaomoreno joaomoreno added this to the July 2023 milestone Jul 4, 2023
src/main.rs Outdated
@@ -100,17 +100,16 @@ fn delete_existing_version(
let root = PathBuf::from(root_path);
directories.push_back(root);

while directories.len() > 0 {
while directories.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

Whoops.

src/main.rs Outdated
Comment on lines 414 to 419
let msg = format!(
"Failed to install Visual Studio Code update.
Updates may fail due to anti-virus software and/or runaway processes. Please try restarting your machine before attempting to update again.
Please read the log file for more information:
{log_path}"
);
Copy link
Member

Choose a reason for hiding this comment

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

This will add weird indentation to the message.

src/process.rs Outdated

pe32.dwSize = mem::size_of::<PROCESSENTRY32W>() as u32;

if Process32FirstW(handle, &mut pe32).is_negative() {
Copy link
Member

Choose a reason for hiding this comment

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

This breaks the logic: BOOL is 1 for true and 0 for false, while is_negative() catches neither case.

src/process.rs Outdated
id: pe32.th32ProcessID,
});

if Process32NextW(handle, &mut pe32).is_negative() {
Copy link
Member

Choose a reason for hiding this comment

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

This breaks the logic: BOOL is 1 for true and 0 for false, while is_negative() catches neither case.

@joaomoreno joaomoreno enabled auto-merge (squash) July 4, 2023 19:01
@joaomoreno joaomoreno merged commit 0748f6b into microsoft:main Jul 4, 2023
3 checks passed
@panekj panekj deleted the feat/windows-sys branch July 4, 2023 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants