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

SES1628 - Add git commit details to version info #1459

Merged

Conversation

AL-Session
Copy link
Collaborator

Contributor checklist

  • I have tested my contribution on these devices:
  • Virtual Pixel 3a, Android 14 API 34
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Adds the first 6 chars of the current git commit hash used to build the app to the "Version: [foo]" details in the Settings activity.

Copy link

@bemusementpark bemusementpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! though, BuildConfigField is perhaps more appropriate.

@@ -7,7 +7,7 @@
<string name="ban">مسدود</string>
<string name="save">ذخیره</string>
<string name="note_to_self">یادداشت به خود</string>
<string name="version_s">نسخه</string>
<string name="version_s">%s نسخه</string>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how did you spot this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Android wouldn't format the string with %s substitutions, so I looked through the translations until I found one that was missing the substitution token - after adding it substitution was allowed. No idea how the code in dev worked without it - but git history of this line says the last modifications was in 2022...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might just be a setting in Android Studio, or a feature of some version of Android Studio?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might just be a setting in Android Studio, or a feature of some version of Android Studio?

Ah, yeah - I did upgrade to Android Studio 2023.3.1 RC 2 the other day to play w/ the Gemini AI while it's in preview (i.e. free) - perhaps that picked it up while previous AS versions won't complain about it.

app/build.gradle Outdated
}
debug {
isDefault true
minifyEnabled false
enableUnitTestCoverage true
resValue("string", "git_hash", getGitHash())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resources are really useful for things that have different values in different languages/configurations.

This is a constant, so it can be a buildConfigField as a constant. Also then we can just declare it once.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image
Done!

Copy link

@bemusementpark bemusementpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, not sure about

  1. Is this just supposed to be on emulators and/or debug build?

  2. I'm not sure the formatting of v1.17.15 (1705 - aef726) is the most readable, I guess there's no designs for this.

@KeeJef
Copy link
Collaborator

KeeJef commented Apr 15, 2024

LGTM, not sure about

  1. Is this just supposed to be on emulators and/or debug build?
  2. I'm not sure the formatting of v1.17.15 (1705 - aef726) is the most readable, I guess there's no designs for this.
  1. This will be a fairly useful piece of information for non debug / emulator builds too, for example if we are doing internal user tests and users are on different builds.
  2. I think the visual display of this is fine for now, its not a highly exposed feature, being at the bottom of the settings.

@AL-Session AL-Session merged commit fbc82d7 into oxen-io:dev May 2, 2024
1 check passed
@AL-Session AL-Session deleted the SES1628_AddGitCommitDetailsToVersionInfo branch May 2, 2024 02:55
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.

3 participants