gnupg-for-java icon indicating copy to clipboard operation
gnupg-for-java copied to clipboard

Published 20 hours ago • verified publisher icon guardianproject

[REVIEW] Some modifications

Open daniele-athome opened this issue 10 years ago • 9 comments

I created this PR to let you review my modifications and eventually create one or more selective PRs to do the actual merges (or, if you like all of them, you might merge this PR as well).

daniele-athome avatar Dec 12 '14 16:12 daniele-athome

Overall looks good. In general, all of the commit messages should describe why you made the changes, because its not obvious. For the most part, the existing commit messages are describing what has changed, but that is visible from reading the code.

eighthave avatar Dec 12 '14 17:12 eighthave

You're right. I will re-apply the changes on your master with improved commit messages.

daniele-athome avatar Dec 13 '14 11:12 daniele-athome

I've reapplied the commits with some explanations. If you think they can suffice, let's comment them here. Merry Christmas by the way :-)

daniele-athome avatar Dec 24 '14 19:12 daniele-athome

@daniele-athome are you planning on doing a new pull request, or shall I cherry-pick these commits?

eighthave avatar Jan 27 '15 09:01 eighthave

Hey, sorry I've been busy. I was planning to rework these things in the short term (2-4 weeks):

  • expose gpgme_set_keylist_mode
  • handle FILE handler (faulty fclose) in some other ways
  • throw UnsupportedOperationException for backwards compatibility

The rest you can pick (at your choice):

  • Maven script (requires manual copy of the .so file though, and it's not very portable - e.g. might not work on Windows, but it's a start)
  • implement GnuPGKey.getTimestamp
  • remove debugging log (some unpolite printf :P)

The only doubt is "return first fingerprint from import". How do you think we should proceed on that?

daniele-athome avatar Jan 27 '15 09:01 daniele-athome

No apologies necessary! Just checking in :-) Your future work stuff sounds very useful. I'll pick the commits you mentioned for now, to get things working on older versions of gpgme.

As for "return first fingerprint from import", why not always return a String[] of 0-inf fingerprints? Doing a String[] in JNI will probably more painful than it should be, as usual, but that seems to me to be the correct API.

eighthave avatar Jan 27 '15 10:01 eighthave

Actually the import method should return an array (or a list) of classes describing the imported result in detail (much like the import struct used in gpgme). But let's start with a fingerprint array of successfully imported keys.

daniele-athome avatar Jan 27 '15 10:01 daniele-athome

returning an array of the imported keys makes sense. But that might now fit in with the rest of the API here. It seems to work in its own odd way, so it might be more confusing if the import doesn't follow that.

eighthave avatar Jan 27 '15 10:01 eighthave

Hello Committers, I tried to build using ant in my windows 8 machine, But facing issue with ant build. I posted a question in Stackoverflow also. Please help me to use GnuPG Java API to work with encryption and decryption. It would be much helpful if you put some examples with encrypt,decrypt,sign and etc. Stackoverflow question here http://stackoverflow.com/questions/30299143/gnupg-for-java-library-ant-build-issue-bug

Sorry for posting comment here.. There is no issue creation option to create issue in this project..

lakshminarayanar avatar May 19 '15 07:05 lakshminarayanar