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

feat: add build number to settings screen #6234

Merged
merged 1 commit into from
Nov 15, 2024
Merged

Conversation

kathaypacific
Copy link
Collaborator

Description

As the title. This will be helpful to identify which version of the nightly build you're on.

Test plan

Simulator Screenshot - iPhone 15 Pro - 2024-11-15 at 10 19 38

Related issues

n/a

Backwards compatibility

Y

Network scalability

If a new NetworkId and/or Network are added in the future, the changes in this PR will:

  • Continue to work without code changes, OR trigger a compilation error (guaranteeing we find it when a new network is added)

Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.98%. Comparing base (e39b981) to head (0c5e25a).
Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #6234   +/-   ##
=======================================
  Coverage   88.98%   88.98%           
=======================================
  Files         737      737           
  Lines       31449    31450    +1     
  Branches     5828     5828           
=======================================
+ Hits        27985    27986    +1     
  Misses       3266     3266           
  Partials      198      198           
Files with missing lines Coverage Δ
src/navigator/SettingsMenu.tsx 94.89% <100.00%> (+0.05%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e39b981...0c5e25a. Read the comment docs.

Copy link
Contributor

@bakoushin bakoushin left a comment

Choose a reason for hiding this comment

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

since it's mostly an engineer-targeted feature, what do you think about adding it under the dev settings (those that appear after many taps on the version)

@kathaypacific kathaypacific added this pull request to the merge queue Nov 15, 2024
@kathaypacific kathaypacific removed this pull request from the merge queue due to a manual request Nov 15, 2024
@kathaypacific
Copy link
Collaborator Author

since it's mostly an engineer-targeted feature, what do you think about adding it under the dev settings (those that appear after many taps on the version)

i agree this information is more for debugging, but i also think it is supplementary information to the app version. i see this pattern of app version (build version) in many apps on my phone so it doesn't seem like it'd cause confusion for users, and giving less reason to activate the debug menu (where you have the dangerous reset app without warning option that you can accidentally tap) seems like a good thing...

@kathaypacific kathaypacific added this pull request to the merge queue Nov 15, 2024
@bakoushin
Copy link
Contributor

giving less reason to activate the debug menu

but what are the reasons for the end user to activate the debug menu? I thought it was only for the engineers (who are aware of its existence 😅)

@kathaypacific kathaypacific removed this pull request from the merge queue due to a manual request Nov 15, 2024
@kathaypacific
Copy link
Collaborator Author

kathaypacific commented Nov 15, 2024

giving less reason to activate the debug menu

but what are the reasons for the end user to activate the debug menu? I thought it was only for the engineers (who are aware of its existence 😅)

this is the context for adding visibility for the build number, i think there are genuine use cases for wanting to know the build number of an app outside of while you're engineering a feature. i suppose you could argue that only engineers have the nightly build, and they should be trusted to toggle the dev menu responsibly (since the debug menu is not enabled by default on any published apps), but this is an extra step that doesn't seem necessary unless there's some harm to exposing this build number alongside the app version? in the event that we accidentally do a public release without bumping the app version, the build number will also become important for debugging

@bakoushin
Copy link
Contributor

i thought it would bloat the UI with seemingly unnecessary information for the end users.
the standard version number is something that users see in the app stores and we could refer to it when communicating about the app.

as for debugging, i noticed that when some debugging/investigation is required, we usually require the end users to send their logs, which include the build number. in the discussion you mentioned, what if the user had the expected build number, but still experiencing the issue? we would ask them then to send the logs, right?

however, within the engineering team, we may want to have a shortcut to quickly reveal the build number without going the full circle with logs.

having that said, I don't want to die on that hill 😃 if it really could help us resolve some issues more quickly, then why not

@kathaypacific
Copy link
Collaborator Author

kathaypacific commented Nov 15, 2024

i thought it would bloat the UI with seemingly unnecessary information for the end users. the standard version number is something that users see in the app stores and we could refer to it when communicating about the app.

as for debugging, i noticed that when some debugging/investigation is required, we usually require the end users to send their logs, which include the build number. in the discussion you mentioned, what if the user had the expected build number, but still experiencing the issue? we would ask them then to send the logs, right?

however, within the engineering team, we may want to have a shortcut to quickly reveal the build number without going the full circle with logs.

having that said, I don't want to die on that hill 😃 if it really could help us resolve some issues more quickly, then why not

I hear the feedback that the build number might just be unnecessary clutter for most users, and I agree that we can usually get this info from support tickets when debugging real user issues. That said, I still feel that having the app version + build number is a more precise way to identify what version of the app someone is using. This is especially helpful for the nightly build, which we rely on heavily to test new features—not just engineers, but also folks like Laura and Denisse. It seems much easier for them to include the build number in issue reports than having to dig into the debug menu (if we agree that generally, exposing the debug menu to non engineers is not desirable), as long as it's not causing too much harm to users to see this information. Given the screen doesn't have a lot of info on it now and the build number is relatively short, I still see it as a worthy tradeoff to have here

@kathaypacific kathaypacific added this pull request to the merge queue Nov 15, 2024
Merged via the queue into main with commit d2a7f64 Nov 15, 2024
15 checks passed
@kathaypacific kathaypacific deleted the kathy/build-number branch November 15, 2024 15:16
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.

2 participants