-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Remove redundant word #14607
Remove redundant word #14607
Conversation
Pull Request for joomla#10971 As requested by @ot2sen and others this PR removes the redundant word successfully from success messages. It's just not needed at all. You can't have a success message that says you "unsuccessfully" did something so there is no need to use the word. This can be merged after 3.7.0 if requested - this is upto @Bakual as commented at joomla#10971
Thanks. |
COM_CONTENTHISTORY_N_ITEMS_DELETED_1="%s history version successfully deleted." | ||
COM_CONTENTHISTORY_N_ITEMS_DELETED="%s history versions successfully deleted." | ||
COM_CONTENTHISTORY_N_ITEMS_DELETED_1="%s history version deleted." | ||
COM_CONTENTHISTORY_N_ITEMS_DELETED="%s history versions deleted." | ||
COM_CONTENTHISTORY_N_ITEMS_KEEP_TOGGLE_1="Successfully changed the keep forever value for %s history version." |
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.
Remove Successfully
and the following line.
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.
No. It makes it correct English to begin with the word. It is not redundant here.
I have tested this item ✅ successfully on 97b3e47 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14607. |
I have tested this item ✅ successfully on 97b3e47 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14607. |
RTC after two successful tests. |
@brianteeman can you fix the conflicts? |
It has a milestone of 3.8 so I really only want to fix conflicts once. If someone can say when it will be merged in to 3.8 I will fix the conflicts (or try to) in time for that |
Cant you just change the base branch? And that we can merge it into 3.8-dev? |
I dont see how to change the base branch? |
Conflicts resolved |
you can do this by using the edit button. but this require the staging and 3.8-dev branch to be in sync (i have just reyed it). I think I can take a look into that after 3.7.0 stable is out. |
base branch changed |
Yes but now your PR contains 40 file changes (most are not realted to your PR) https://github.com/joomla/joomla-cms/pull/14607/files |
ah - oops - changed back to staging now. |
what is with the "COM_CONTACT_N_ITEMS_(UN)FEATURED" Tags? |
conflicts resolved - it was a pr you just merged |
thanks for merging @Bakual pinging you to do whatever it is you wanted to do on crowdin |
Hope it worked :) |
Can we revert this PR, because many tests are broken because this change. We should change this later, not on minor versions. |
what tests? |
Codeception tests. |
there are 17 changes in total to make and it will take you about 5 minutes |
This change is absolutely fine for patch versions (it's not even a minor version). On a sidenote: As long as those tests don't run automatically on Pull Requests, we cant make decisions based on those tests anyway. |
But our componens/modules/plugins/templates are running on the pr's, so we are having failed tests now. I will improve it, but it was better todo on J3.8, now we have b/c, some 3rth parties will have this issue. |
Why do we have a test suite that is supposedly testing the core package not integrated into this repository in any way (either the tests living in our tests directory or hooked into the CI process)? Regardless of when this PR was accepted and where the tests live, it all would have to be updated anyway. I don't see a strong argument for reverting language improvements because some tooling requires updates to work with those changes. |
what is the point in having a test suite if its not integrated. seems to me that it means that anyoone working on the test suite is wasting their time |
It's no backward compatiblity break if language strings get improved. Nothing will break except some tests which expects a specific string. But that's a "false positive" as in production nothing wil break at all. |
And in terms of updating tests it's no worse than having to update a test validating HTML output when the method being tested gets updated. |
The tests could have been updated by now. ;)
I would have done it myself if I wasn't on a train 🚂 crossing a border
…On 24 May 2017 2:49 p.m., "Michael Babker" ***@***.***> wrote:
And in terms of updating tests it's no worse than having to update a test
validating HTML output when the method being tested gets updated.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#14607 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABPH8UEENiEhh_Jy6LtUHv-mO1uyXBuhks5r9DVKgaJpZM4MckJo>
.
|
Pull Request for #10971
As requested by @ot2sen and others this PR removes the redundant word successfully from success messages. It's just not needed at all. You can't have a success message that says you "unsuccessfully" did something so there is no need to use the word.
Note for readability where a sentence began with the word successfully I left it there.
This can be merged after 3.7.0 if requested - this is upto @Bakual as commented at #10971