-
-
Notifications
You must be signed in to change notification settings - Fork 935
docs: Fix typos and settings order #2785
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥞📝 LGTM!
Enabling this will allow you to change the patch selection | ||
> ⚠️ Warning | ||
> Changing the selection may cause unexpected issues. | ||
> Unless you know what you are doing, it is recommended to keep this disabled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 1.26.0-dev.7, this option is enabled by default. Is that a bug, or does this warning need to be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is enabled by default yes, this is not "expected", I already notified the team in discord.
Please look into #2780 (comment) as well. |
Settings aren't considered final, this PR should be postponed |
<string name="auto_update_description">Automatically update when a new version is available</string> | ||
<string name="patches_prereleases">Use pre-releases</string> | ||
<string name="patches_prereleases_description">Use pre-release version of %s</string> | ||
<string name="patches_prereleases_description">Use pre-release versions of %s</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<string name="patches_prereleases_description">Use pre-release versions of %s</string> | |
<string name="patches_prereleases_description">Use pre-release version of %s</string> |
Isn't it better to use version at both places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case the toggle titles will also need to be singular. Currently they are plural ("Use pre-releases")
Also, I think "versions" better reflects that the patches and Manager are constantly updated. There isn't a single pre-release version. Rather, it is constantly changing. Not sure if I'm explaining myself well. I understand your perspective too 😅
Isn't it best to have it reflect the current state of settings? If the settings are changed, a new PR can be opened in the future. I wish not to have this PR hang open, because then I will be responsible to reflect all future changes and mistakes. I don't like putting workloads on myself, so I was hoping this would quickly be merged or closed. Same for my other PR: #2791 |
I don't think this PR should be postponed, this PR is relating to the current branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥞 LGTM! (latest versions)
No description provided.