Skip to content
This repository has been archived by the owner on Jul 26, 2023. It is now read-only.

Add cabinet API #471

Merged
merged 6 commits into from
Jul 1, 2020
Merged

Add cabinet API #471

merged 6 commits into from
Jul 1, 2020

Conversation

qmfrederik
Copy link
Contributor

  • FDICreate
  • FDICopy
  • FDIDestroy

- FDICreate
- FDICopy
- FDIDestroy
Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

I see your PR is a draft so perhaps I'm jumping the gun, but here is some feedback.
Thanks so much for contributing. This is a lot of good stuff!

src/Cabinet/Cabinet+CpuType.cs Outdated Show resolved Hide resolved
src/Cabinet/Cabinet.cs Outdated Show resolved Hide resolved
src/Cabinet/Cabinet.cs Outdated Show resolved Hide resolved
src/Cabinet/Cabinet.cs Outdated Show resolved Hide resolved
src/Kernel32/Kernel32.cs Show resolved Hide resolved
src/PInvoke.sln Show resolved Hide resolved
@qmfrederik
Copy link
Contributor Author

@AArnott Thanks for the review! I addressed your comments. I mainly marked the PR as draft because there are a bunch of other PRs open and I didn't want to flood you with PRs 😄 .

Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Very close

README.md Outdated
@@ -95,6 +95,7 @@ user32.dll |`PInvoke.User32` | [![NuGet](https://buildstats.info/nuget/PInvok
userenv.dll |`PInvoke.Userenv` | [![NuGet](https://buildstats.info/nuget/PInvoke.Userenv)](https://www.nuget.org/packages/PInvoke.Userenv)|Windows User Environment
uxtheme.dll |`PInvoke.UxTheme` | [![NuGet](https://buildstats.info/nuget/PInvoke.UxTheme)](https://www.nuget.org/packages/PInvoke.UxTheme)|[Windows Visual Styles][UxTheme]
WtsApi32.dll |`PInvoke.WtsApi32`| [![NuGet](https://buildstats.info/nuget/PInvoke.WtsApi32)](https://www.nuget.org/packages/PInvoke.WtsApi32)|[Windows Remote Desktop Services][WtsApi32]
cabinet.dll |`PInvoke.Cabinet` | [![NuGet](https://buildstats.info/nuget/PInvoke.Cabinet)](https://www.nuget.org/packages/PInvoke.Cabinet)|[Cabinet API Functions][Cabinet]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please keep the table sorted. (if there's a pre-existing sorting error, feel free to fix it)

public delegate uint FNSEEK(int hf, int dist, SeekOrigin seektype);

[UnmanagedFunctionPointer(CallingConvention.Cdecl)]
public delegate int PFNNOTIFY(NOTIFICATIONTYPE fdint, NOTIFICATION fdin);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You've done a great job with docs everywhere else. Is this one missing docs because no docs exist, or was it an oversight? This wouldn't be a reason to deny merging a PR, but I wanted to give you a chance to fill it in.

Comment on lines 168 to 169
[Friendly(FriendlyFlags.Out)]
ERF* perf);
Copy link
Collaborator

Choose a reason for hiding this comment

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

One line per parameter, please.

Copy link
Collaborator

@AArnott AArnott left a comment

Choose a reason for hiding this comment

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

Thanks

@qmfrederik qmfrederik marked this pull request as ready for review July 1, 2020 14:30
@qmfrederik qmfrederik mentioned this pull request Jul 1, 2020
@AArnott AArnott merged commit df9373c into dotnet:master Jul 1, 2020
@qmfrederik qmfrederik deleted the features/cabinet branch July 1, 2020 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants