search

sailfishos / sailfish-secrets

BSD 3-Clause "New" or "Revised" License
24 stars 15 forks source link

[WiP] GObject API implementation for preliminary review #147

Closed inzanity closed 5 years ago

chriadam commented 5 years ago

Purely cosmetic, but I don't know if we use short-form naming (like sf) anywhere, possibly "sailfish" could be more consistent even though more verbose.

inzanity commented 5 years ago

The problem with long prefixes in C is that they are repeated. Everywhere. I'm not against s///g'in it entirely, but to me it makes sense to keep it short. Granted the function names etc are already long, and the added length doesn't add much, but IMO things are more readable when the common prefix is shorter.

monich commented 5 years ago

I would consider shortening sf_secrets_ prefix to just sfs_ :)

e.g.

sfs_collection_get_encryption_plugin_name
sf_secrets_collection_get_encryption_plugin_name
sailfish_secrets_collection_get_encryption_plugin_name
inzanity commented 5 years ago

I was thinking that too, not just for shortness, but also because then the TYPE in macro names would sit nicely in more logical position. :)

chriadam commented 5 years ago

The Crypto API is looking good, really nice work. Obviously a few more things to go (e.g. changing the secrets API a bit, supporting (symmetric) key derivation and (asymmetric) key generation parameters in the crypto API, and unit tests for everything, but definitely looking great!

rainemak commented 5 years ago

What's the motivation behind variant to bool change in the dbus interface? Pondering who much we'll break with this?

rainemak commented 5 years ago

That being said once commits are cleaned up we should get this integrated.

rainemak commented 5 years ago

Usually we use 4spaces per indentation level, now there are tabs. Could you replace those.

monich commented 5 years ago

Usually we use 4spaces per indentation level, now there are tabs. Could you replace those.

Let me leave my 2¢ here. I don't like tabs at all and generally try to avoid them but at the same time I don't feel good about forcing people to use spaces for indentation. As long as it's consistent within the component (i.e. tabs are not mixed with spaces) I would leave it the way it is in the form which is most comfortable to the person who is most likely going to maintain this code. Programming is a lot more fun if you like the code that you're writing which (hopefully) translates into quality. And the opposite doesn't translate into quality, that's for sure :)

chriadam commented 5 years ago

What happened to cause all of the conflicts here?

rainemak commented 5 years ago

@chriadam see https://github.com/sailfishos/sailfish-secrets/pull/151 . @inzanity prepared a cleanup commit set.

This should be closed as well.

inzanity commented 5 years ago

Obsoleted by #151