openmobilehub / android-omh-auth

Apache License 2.0
4 stars 1 forks source link

Inconsistencies in Naming Conventions for Provider-Specific Variables in local.properties #32

Closed dzuluaga closed 8 months ago

dzuluaga commented 8 months ago

I've encountered some inconsistencies in the naming conventions used for provider-specific variables within the local.properties file of our project. These discrepancies can lead to confusion and make the configuration process less intuitive, especially for new developers or those transitioning between different parts of our project and the respective service provider consoles/documentation. I'd like to highlight a couple of examples and suggest a review of our naming conventions to ensure consistency across all modules, including Maps and Storage.

  1. DropBox Variable Naming:

    • Current Name: DROPBOX_APP_ID
    • Suggested Name: DROPBOX_APP_KEY
    • Reason: The DropBox documentation and console refer to this as the "App Key" rather than "App ID". Aligning our variable name with DropBox's terminology would enhance clarity and reduce potential confusion.
  2. Microsoft Variable Naming:

    • Current Name: KEYSTORE_HASH
    • Suggested Name: MICROSOFT_SIGNATURE_HASH
    • Reason: In the context of Microsoft services, the variable represents what is known in their console/documentation as the "Signature Hash". Renaming our variable to MICROSOFT_SIGNATURE_HASH would better reflect its purpose and maintain consistency with Microsoft's naming convention.

Suggested Action: I propose that we conduct a thorough review of the naming conventions for all provider-specific variables in the local.properties file, especially focusing on the Maps and Storage modules. This review should aim to ensure that our variable names accurately reflect the terminology used by the respective service providers. Such alignment would not only improve the developer experience but also enhance the maintainability and readability of our configuration files.

Benefits:

I look forward to hearing your thoughts on this matter and discussing potential steps forward.

Thank you for considering this improvement.

andrei-zgirvaci commented 8 months ago

Thanks Diego for pointing this out! I have changed the Dropbox secret from DROPBOX_APP_ID to DROPBOX_APP_KEY in https://github.com/openmobilehub/android-omh-auth/commit/a19c483d5d346016d76ebb24bbba9e0690e5cc16.

Regarding Microsoft, I wouldn't suggest changing the naming. KEYSTORE_HASH is the hash that you get from your debug or release Keystore. In my opinion, changing it to MICROSOFT_SIGNATURE_HASH will lead to more confusion.

dzuluaga commented 8 months ago

Thanks for taking my feedback. @andrei-zgirvaci can you tell me where KEYSTORE_HASH variable is used besides Microsoft? As far as I know this variable is only used for Microsoft. If it's only used for Microsoft, including the provider using it, helps clarify its purpose. Please advise. Thanks

andrei-zgirvaci commented 8 months ago

Yes, it's only used by Microsoft. I have changed the secret from KEYSTORE_HASH to MICROSOFT_SIGNATURE_HASH in https://github.com/openmobilehub/android-omh-auth/pull/39/commits/586359add898e8d68491e62f2664b20df1bce93c.