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

Add version output to AboutFragment #169

Merged
merged 4 commits into from
Oct 6, 2024

Conversation

DeKaN
Copy link
Contributor

@DeKaN DeKaN commented Oct 3, 2024

Contributor checklist


Description

Adds current version text to AboutFragment. Clicking on it opens Github Releases page. Version text is generated from property android.defaultConfig.versionName in build.gradle.kts

Related issue

Copy link

github-actions bot commented Oct 3, 2024

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

  • The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution

    • The contributor's name and icon in remote commits should be the same as what appears in the PR
    • If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local Scribe-Android repo
  • The linting and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

@@ -27,7 +27,7 @@
<string name="app.about.feedback.rate_conjugate">Rate Scribe Conjugate</string>
<string name="app.about.feedback.rate_scribe">Rate Scribe</string>
<string name="app.about.feedback.title">Feedback and support</string>
<string name="app.about.feedback.version">Version</string>
<string name="app.about.feedback.version">Version %1$s</string>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be updated in Scribe-i18n repo also

Copy link
Member

@angrezichatterbox angrezichatterbox Oct 4, 2024

Choose a reason for hiding this comment

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

@DeKaN Could you remove these changes for now where the assets folder values are being changed? It would mess up the git subtree. @andrewtavis Should we make the text for now be taken from strings.xml which Android has given by default until the string is changed?

Copy link
Member

@angrezichatterbox angrezichatterbox left a comment

Choose a reason for hiding this comment

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

Thank you so much for the PR @DeKaN. Could u remove the changes from the assets and temporarily add the value to the strings.xml file only and then the PR would be perfect

@DeKaN
Copy link
Contributor Author

DeKaN commented Oct 4, 2024

@angrezichatterbox Done

@andrewtavis andrewtavis added the hacktoberfest-accepted Accepted as a part of Hacktoberfest label Oct 4, 2024
Copy link
Member

@angrezichatterbox angrezichatterbox left a comment

Choose a reason for hiding this comment

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

Could you run the app ones so that changes to values-es would be retracted? Also, could you add the version string to strings.xml rather than string.xml? So it is like every time the app is run the values in string.xml files get changed.

@DeKaN
Copy link
Contributor Author

DeKaN commented Oct 5, 2024

Yes, Gradle overwrites string.xml by version from assets (that's why I added changes there).
If I move Version string to strings.xml, I have to use another resource id and the string will be excluded from your i18n. Are you sure?

@angrezichatterbox
Copy link
Member

angrezichatterbox commented Oct 5, 2024

Yes, Gradle overwrites string.xml by version from assets (that's why I added changes there). If I move Version string to strings.xml, I have to use another resource id and the string will be excluded from your i18n. Are you sure?

The reference remains the same you could call using R.string.name_of_string or @string/string_name. The value would get changed during the build if it's added to string.xml. For now, could u add it to strings.xml. Let the name be the same.

@DeKaN
Copy link
Contributor Author

DeKaN commented Oct 5, 2024

Resources must have unique ids. So string with id app.about.feedback.version must be removed from all string.xml files, if you want to put it to strings.xml. But you don't want to change files in assets.
I can add it with another id, but you'll get this conflict later when you update the id in i18n repo

@angrezichatterbox
Copy link
Member

Resources must have unique ids. So string with id app.about.feedback.version must be removed from all string.xml files, if you want to put it to strings.xml. But you don't want to change files in assets. I can add it with another id, but you'll get this conflict later when you update the id in i18n repo

I forgot that there was a Version string. For not just add a new string to strings.xml with the resource id of your choice and then use the string.

@DeKaN
Copy link
Contributor Author

DeKaN commented Oct 6, 2024

Done

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, @DeKaN, and also for your two's collaboration 😊 Just tested and it's working fine. We can figure out the specifics of the text once we bring the Scribe-i18n subtree in next time :)

@andrewtavis andrewtavis merged commit de3f3a7 into scribe-org:main Oct 6, 2024
1 check passed
@DeKaN DeKaN deleted the about-adding-version branch October 6, 2024 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted as a part of Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants