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

Change Error to Other on settingsPage #36776

Merged
merged 1 commit into from
Jan 20, 2020
Merged

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jan 17, 2020

Description

If we change Error to Other (which is already a translated string) then we avoid the translation problem. IMO "Other" is an OK title for this "Additional" settings page when it is empty.

Related Issue

Motivation and Context

Confusion should be avoided.

How Has This Been Tested?

Manual

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@pmaier1
Copy link
Contributor

pmaier1 commented Jan 17, 2020

Isn't this the same as #36775 but with other wording?

@codecov
Copy link

codecov bot commented Jan 17, 2020

Codecov Report

Merging #36776 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #36776   +/-   ##
=========================================
  Coverage     64.68%   64.68%           
  Complexity    19121    19121           
=========================================
  Files          1269     1269           
  Lines         74778    74778           
  Branches       1320     1320           
=========================================
  Hits          48372    48372           
  Misses        26018    26018           
  Partials        388      388
Flag Coverage Δ Complexity Δ
#javascript 54.12% <ø> (ø) 0 <ø> (ø) ⬇️
#phpunit 65.85% <ø> (ø) 19121 <ø> (ø) ⬇️
Impacted Files Coverage Δ Complexity Δ
settings/templates/settingsPage.php 0% <ø> (ø) 0 <0> (ø) ⬇️

Continue to review full report at Codecov.

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

@phil-davis
Copy link
Contributor Author

phil-davis commented Jan 17, 2020

Isn't this the same as #36775 but with other wording?

yes, the "advantage" is that the word "Other" is already used elsewhere and exists in translations. So there is no need to wait for people to do translations on Transifex.

It's just my idea of a way to be able to "fix" this without the hassle of having to update all the translations.

Copy link
Contributor

@pmaier1 pmaier1 left a comment

Choose a reason for hiding this comment

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

Thx

@phil-davis phil-davis merged commit efd8ba8 into master Jan 20, 2020
@delete-merged-branch delete-merged-branch bot deleted the other-for-empty-panel branch January 20, 2020 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants