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

Provide support for the Windows RAS32 API #267

Merged
merged 3 commits into from
Sep 25, 2013
Merged

Provide support for the Windows RAS32 API #267

merged 3 commits into from
Sep 25, 2013

Conversation

kc7bfi
Copy link
Contributor

@kc7bfi kc7bfi commented Sep 23, 2013

I've provided new code to add support for Windows RAS32 API.

@dblock
Copy link
Member

dblock commented Sep 23, 2013

This looks great. Can you please add a line into CHANGES.md? Thx.

@dblock
Copy link
Member

dblock commented Sep 23, 2013

There're also a lot of tests missing, for things like RasGetCredentials. It's not always obvious how to write a test for something like this, but a negative test is useful: call it with some invalid parameter and expect that it returns an expected error, for example.

@kc7bfi
Copy link
Contributor Author

kc7bfi commented Sep 23, 2013

OK, I've added the line in CHANGES.md. I wasn't sure how to generate
tests without actually creating or dialing RAS phone book entries.

David R Robison
Open Roads Consulting, Inc.
103 Watson Road, Chesapeake, VA 23320
phone: (757) 546-3401
e-mail: drrobison@openroadsconsulting.com
web: http://openroadsconsulting.com
blog: http://therobe.blogspot.com
book:
http://www.xulonpress.com/bookstore/bookdetail.php?PB_ISBN=9781597816526

On Monday, September 23, 2013 4:01:20 PM, Daniel Doubrovkine (dB.)
wrote:

There're also a lot of tests missing, for things like
|RasGetCredentials|. It's not always obvious how to write a test for
something like this, but a negative test is useful: call it with some
invalid parameter and expect that it returns an expected error, for
example.


Reply to this email directly or view it on GitHub
#267 (comment).

This email communication (including any attachments) may contain confidential and/or privileged material intended solely for the individual or entity to which it is addressed.
If you are not the intended recipient, please delete this email immediately.

@kc7bfi
Copy link
Contributor Author

kc7bfi commented Sep 23, 2013

OK, I've added more tests, I think the pull request should be good now.
David

David R Robison
Open Roads Consulting, Inc.
103 Watson Road, Chesapeake, VA 23320
phone: (757) 546-3401
e-mail: drrobison@openroadsconsulting.com
web: http://openroadsconsulting.com
blog: http://therobe.blogspot.com
book:
http://www.xulonpress.com/bookstore/bookdetail.php?PB_ISBN=9781597816526

On Monday, September 23, 2013 4:05:37 PM, David R Robison wrote:

OK, I've added the line in CHANGES.md. I wasn't sure how to generate
tests without actually creating or dialing RAS phone book entries.

David R Robison
Open Roads Consulting, Inc.
103 Watson Road, Chesapeake, VA 23320
phone: (757) 546-3401
e-mail: drrobison@openroadsconsulting.com
web: http://openroadsconsulting.com
blog: http://therobe.blogspot.com
book:
http://www.xulonpress.com/bookstore/bookdetail.php?PB_ISBN=9781597816526

On Monday, September 23, 2013 4:01:20 PM, Daniel Doubrovkine (dB.) wrote:

There're also a lot of tests missing, for things like
|RasGetCredentials|. It's not always obvious how to write a test for
something like this, but a negative test is useful: call it with some
invalid parameter and expect that it returns an expected error, for
example.


Reply to this email directly or view it on GitHub
#267 (comment).

This email communication (including any attachments) may contain confidential and/or privileged material intended solely for the individual or entity to which it is addressed.
If you are not the intended recipient, please delete this email immediately.

@dblock dblock merged commit 8a4a7e1 into java-native-access:master Sep 25, 2013
@dblock
Copy link
Member

dblock commented Sep 25, 2013

Merged, thanks for contributing.

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.

None yet

2 participants