Using shared copy of registryvalueKind #31922
Conversation
| <Link>Common\Interop\Windows\Interop.RegistryOptions.cs</Link> | ||
| </Compile> | ||
| <Compile Include="$(CommonPath)\CoreLib\Interop\Windows\Kernel32\Interop.RegistryValues.cs"> | ||
| <Link>Common\CoreLib\Interop\Windows\Kernel32\Interop.RegistryValues.cs</Link> |
There was a problem hiding this comment.
All registry APIs are defined in Advapi32. The related constants should be defined in Advapi32 as well. I do not think we want to be moving these to Kernel32.
(I am sorry that I have not caught this in the previous PR.)
There was a problem hiding this comment.
np i will move everything to kernel32
There was a problem hiding this comment.
Did you mean to say Advapi32 ?
|
@dotnet-bot test Linux x64 Release Build |
| <Compile Include="$(MSBuildThisFileDirectory)System\Numerics\Vector_Operations.cs" /> | ||
| </ItemGroup> | ||
| <ItemGroup Condition="$(TargetsWindows)"> | ||
| <Compile Include="$(MSBuildThisFileDirectory)Interop\Windows\Advapi32\Interop.RegistryValues.cs" /> |
There was a problem hiding this comment.
Can this be in the block with other registry APIs?
There was a problem hiding this comment.
This file was earlier included irrespective of the value of enableWinRt variable where as all other registry files are include is this property is not true ?
is it fine to move there ?
There was a problem hiding this comment.
It should be fine. I do not see any problem with it.
| internal const int STANDARD_RIGHTS_READ = READ_CONTROL; | ||
| internal const int STANDARD_RIGHTS_WRITE = READ_CONTROL; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think it may be better to move this whole file to CoreLib, and not split it into a bunch of little files. All these constants are likely going to be used together. They are needed in CoreLib - they are under different names in CoreLib right now.
| internal const int STANDARD_RIGHTS_WRITE = READ_CONTROL; | ||
| } | ||
|
|
||
| internal partial class RegistryValues |
There was a problem hiding this comment.
Nit: You can make all these static while you are on it.
|
@dotnet-bot test Linux x64 Release Build |
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| internal partial class Interop | ||
| { |
There was a problem hiding this comment.
This file has more than RegistryOptions. Call it Interop.Registry.Constants.cs ?
|
LGTM modulo one nit. |
|
@Anipik CoreLib mirror seems to be stuck. |
* using local copy of registryvaluekind and advapi32 * Moving complete file to shared * name changed Signed-off-by: dotnet-bot <[email protected]>
|
there were some merge conflicts due to da6117f#diff-a5e9bf1aea7b21e5dc1325dc67d264fc |
* using local copy of registryvaluekind and advapi32 * Moving complete file to shared * name changed Signed-off-by: dotnet-bot <[email protected]>
* using local copy of registryvaluekind and advapi32 * Moving complete file to shared * name changed Signed-off-by: dotnet-bot <[email protected]>
* using local copy of registryvaluekind and advapi32 * Moving complete file to shared * name changed Signed-off-by: dotnet-bot <[email protected]>
* using local copy of registryvaluekind and advapi32 * Moving complete file to shared * name changed Commit migrated from dotnet/corefx@39c4ddf
4 participants
No description provided.