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 the wording in the additional tab to not confuse users #36775

Merged
merged 1 commit into from
May 29, 2020

Conversation

ChrisEdS
Copy link
Contributor

Description

This PR changes two strings that caused confusion in the additional tab.

Related Issue

Motivation and Context

Confusion should be avoided.

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

@ChrisEdS ChrisEdS added this to the development milestone Jan 16, 2020
@ChrisEdS ChrisEdS self-assigned this Jan 16, 2020
@ChrisEdS ChrisEdS requested a review from pmaier1 January 16, 2020 23:29
Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

and needs a changelog
has been done

@phil-davis phil-davis force-pushed the fix-additonal-wording branch from 6a82bef to 40778b7 Compare January 17, 2020 04:13
@owncloud owncloud deleted a comment from update-docs bot Jan 17, 2020
@codecov
Copy link

codecov bot commented Jan 17, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #36775   +/-   ##
=========================================
  Coverage     64.92%   64.92%           
  Complexity    19151    19151           
=========================================
  Files          1267     1267           
  Lines         74902    74902           
  Branches       1331     1331           
=========================================
  Hits          48633    48633           
  Misses        25877    25877           
  Partials        392      392           
Flag Coverage Δ Complexity Δ
#javascript 54.14% <ø> (ø) 0.00 <ø> (ø)
#phpunit 66.13% <ø> (ø) 19151.00 <ø> (ø)
Impacted Files Coverage Δ Complexity Δ
settings/templates/settingsPage.php 0.00% <ø> (ø) 0.00 <0.00> (ø)

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 5ee4168...12e8fea. Read the comment docs.

@phil-davis
Copy link
Contributor

I added 1 word and added a changelog.
Tested manually - the text looks fine to me on the webUI.
@pmaier1 please review

@phil-davis phil-davis requested a review from micbar January 17, 2020 05:50
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.

Text is fine. Thanks for taking care everybody 👌

Copy link
Contributor

@micbar micbar left a comment

Choose a reason for hiding this comment

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

Needs to be decided when to merge.

Currently it would go into 10.4 but we didn't update the translations.

@phil-davis
Copy link
Contributor

Note: PR #36776 changed the word "Error" to "Other" on that page. So that "fixed" it for 10.4.

I rebased this just now.

After 10.4 is released, someone can decide if they would like to have the words in this PR, and merge it.

@micbar
Copy link
Contributor

micbar commented Jan 24, 2020

@phil-davis thanks. I think everybody agreed on the solution.

@mmattel
Copy link
Contributor

mmattel commented Feb 13, 2020

Ping ?

@phil-davis
Copy link
Contributor

This is waiting until after 10.4.0 is released. 10.4.0 changed the word "Error" to "Other", which at least gets rid of the misleading word "Error".

If we had realised how long it would be until we got 10.4.0 released, then we could have merged this and the new translations would have had plenty of time to be done. Anyway, too late for that now.

@phil-davis phil-davis force-pushed the fix-additonal-wording branch 2 times, most recently from ea2a2bb to bf0aefa Compare February 28, 2020 11:23
@phil-davis
Copy link
Contributor

I rebased this and adjusted the words of the changelog entry.
With 10.4.0 release "real soon now", we could consider merging this next week for 10.4.1

@phil-davis phil-davis force-pushed the fix-additonal-wording branch from bf0aefa to 12e8fea Compare April 14, 2020 11:11
@phil-davis
Copy link
Contributor

This missed out on 10.4.1
If we want it, then we could merge it this week (e.g. straight after 10.4.1 branch gets merged back...). And that will then give time to do translations before the next release.

@phil-davis
Copy link
Contributor

Nobody merged this.
@micbar if we merge this now, will necessary translations have time to happen and get back from Transifex to core master before an official 10.5.0 branch is created?

@micbar micbar merged commit b2e4c07 into master May 29, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix-additonal-wording branch May 29, 2020 11:20
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.

5 participants