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

HKEY_CURRENT_USER_LOCAL_SETTINGS should be supported #1336

Conversation

Dani-Hub
Copy link
Contributor

Starting with Windows 7, Windows added the additional HKEY constant HKEY_CURRENT_USER_LOCAL_SETTINGS (see predefined-key) whose underlying integral value has the value 0x80000007 (Source: Visual Studio 2017, header <winreg.h>. This constant is currently missing in the existing set of HKEY_* constants in com.sun.jna.platform.win32.WinReg. This adds the new static field HKEY_CURRENT_USER_LOCAL_SETTINGS to the set of the other HKEY constants in WinReg.

Copy link
Contributor

@dbwiddis dbwiddis 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. Please add an entry to CHANGES

@dbwiddis dbwiddis linked an issue Apr 15, 2021 that may be closed by this pull request
@dbwiddis
Copy link
Contributor

HI, @Dani-Hub I'm ready to merge your three pull requests, but need some admin changes from you first:

  • Please change your git settings to include your full name along with your email, rather than just an alias
  • Please add a change log entry for each update to CHANGES.MD. For example:
* [#1336](https://github.com/java-native-access/jna/pull/1336): Add `HKEY_CURRENT_USER_LOCAL_SETTINGS` to `c.s.j.p.win32.WinReg` - [@Dani-Hub](https://github.com/Dani-Hub).

@Dani-Hub
Copy link
Contributor Author

Hi,
Re: "Please change your git settings to include your full name along with your email, rather than just an alias"
Interesting, you are the first project where I had provided such a contribution with that requirement. I'm actually unaware where or how to perform that change. Can you provide a hint to me?
In regard to the log entry changes: Sure, I can do that. Presumably not today, though.

@dbwiddis
Copy link
Contributor

Some instructions here. To change it for all commits anywhere:

git config --global user.name "Your Name"

There are other ways to change for just a single repository or a single commit if desired.

There are a few reasons why this is a good practice and why we do that here:

  • Code contributions are copyrightable and a full name makes it more clear there's a person there.
  • An email address (which you do have) helps in reaching out to contributors if there is a decision to change the license from the one existing when the code was contributed.
  • Having a consistent name and email across commits from different sources helps when looking at commit history and identifying the same user.

@Dani-Hub
Copy link
Contributor Author

Dani-Hub commented Apr 18, 2021

OK, I have now changed user.name and user.email for the local repository where my contribution came from. It is not clear to me, though, when this manifests here. I can of-course just make a pull-request of the changes.md and we'll see what happens.

@dbwiddis
Copy link
Contributor

If you squash the commits it should take the later settings. Or just add a new commit and I can squash them. Thanks!

@dbwiddis
Copy link
Contributor

To be clear, just add a new commit to the branch you've already submitted for this PR. You do not need to do a new PR.

@Dani-Hub
Copy link
Contributor Author

Hmmh, do you notice a difference? I'm not, albeit I double-checked that for that local repository user.name and user.email have been adjusted to the intended values.

@Dani-Hub
Copy link
Contributor Author

I don't understand what this build failure is going to tell me: https://travis-ci.org/github/java-native-access/jna/builds/767460410. Could you please help?

@dbwiddis
Copy link
Contributor

Don't worry about that build failure, it has nothing to do with your commit. It may be a transient error.

@dbwiddis
Copy link
Contributor

Hmmh, do you notice a difference?

Yes, your most recent commit has your full name. I'll squash the commits and merge this PR. Can you do the same for the other two PRs?

@Dani-Hub
Copy link
Contributor Author

Sure, will do in a minute - thanks for the guidance!

Starting with Windows 7, Windows added the additional HKEY constant HKEY_CURRENT_USER_LOCAL_SETTINGS (see predefined-key) whose underlying integral value has the value 0x80000007 (Source: Visual Studio 2017, header <winreg.h>. This constant is currently missing in the existing set of HKEY_* constants in com.sun.jna.platform.win32.WinReg. This adds the new static field HKEY_CURRENT_USER_LOCAL_SETTINGS to the set of the other HKEY constants in WinReg.
@dbwiddis dbwiddis force-pushed the #1332-HKEY_CURRENT_USER_LOCAL_SETTINGS-should-be-supported branch from b15e37f to 4b4df23 Compare April 18, 2021 16:20
@dbwiddis
Copy link
Contributor

BTW the travis test failure is because the download source of the binaries changed their HTML which breaks our script parsing. I'll fix that in a different PR. You can ignore it.

@dbwiddis dbwiddis merged commit b21196e into java-native-access:master Apr 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HKEY_CURRENT_USER_LOCAL_SETTINGS should be supported
2 participants