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

Remove redundant word #14607

Merged
merged 3 commits into from
May 22, 2017
Merged

Remove redundant word #14607

merged 3 commits into from
May 22, 2017

Conversation

brianteeman
Copy link
Contributor

@brianteeman brianteeman commented Mar 14, 2017

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

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
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Mar 14, 2017
@Bakual Bakual added this to the Joomla 3.8.0 milestone Mar 14, 2017
@Bakual
Copy link
Contributor

Bakual commented Mar 14, 2017

Thanks.
I have added the 3.8.0 milestone for now.

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."
Copy link
Contributor

@Quy Quy Apr 16, 2017

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.

Copy link
Contributor Author

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.

@Quy
Copy link
Contributor

Quy commented Apr 23, 2017

I have tested this item ✅ successfully on 97b3e47

Code review.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/14607.

@alikon
Copy link
Contributor

alikon commented Apr 24, 2017

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.

@joomla-cms-bot joomla-cms-bot removed Language Change This is for Translators PR-staging labels Apr 24, 2017
@joomla-cms-bot joomla-cms-bot removed this from the Joomla 3.8.0 milestone Apr 24, 2017
@ghost
Copy link

ghost commented Apr 24, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 24, 2017
@zero-24
Copy link
Contributor

zero-24 commented Apr 24, 2017

@brianteeman can you fix the conflicts?

@brianteeman
Copy link
Contributor Author

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

@zero-24
Copy link
Contributor

zero-24 commented Apr 24, 2017

Cant you just change the base branch? And that we can merge it into 3.8-dev?

@brianteeman
Copy link
Contributor Author

I dont see how to change the base branch?

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Apr 24, 2017
@brianteeman
Copy link
Contributor Author

Conflicts resolved

@zero-24 zero-24 changed the base branch from staging to 3.8-dev April 24, 2017 08:56
@zero-24 zero-24 changed the base branch from 3.8-dev to staging April 24, 2017 08:56
@zero-24
Copy link
Contributor

zero-24 commented Apr 24, 2017

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.

@zero-24 zero-24 self-assigned this Apr 24, 2017
@brianteeman brianteeman changed the base branch from staging to 3.8-dev April 24, 2017 09:13
@brianteeman
Copy link
Contributor Author

base branch changed

@zero-24
Copy link
Contributor

zero-24 commented Apr 24, 2017

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

@brianteeman brianteeman changed the base branch from 3.8-dev to staging April 24, 2017 09:18
@brianteeman
Copy link
Contributor Author

ah - oops - changed back to staging now.

@rdeutz rdeutz added this to the Joomla 3.7.2 milestone May 4, 2017
@rdeutz rdeutz modified the milestones: Joomla 3.7.2, Joomla 3.7.3 May 18, 2017
@rdeutz
Copy link
Contributor

rdeutz commented May 22, 2017

what is with the "COM_CONTACT_N_ITEMS_(UN)FEATURED" Tags?

@brianteeman
Copy link
Contributor Author

conflicts resolved - it was a pr you just merged

@rdeutz rdeutz merged commit 8652ad1 into joomla:staging May 22, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 22, 2017
@brianteeman
Copy link
Contributor Author

thanks for merging

@Bakual pinging you to do whatever it is you wanted to do on crowdin

@brianteeman brianteeman deleted the redundant branch May 22, 2017 20:27
@Bakual
Copy link
Contributor

Bakual commented May 22, 2017

Hope it worked :)

@810
Copy link
Contributor

810 commented May 24, 2017

Can we revert this PR, because many tests are broken because this change. We should change this later, not on minor versions.

@brianteeman
Copy link
Contributor Author

what tests?

@810
Copy link
Contributor

810 commented May 24, 2017

Codeception tests.

See: joomla-projects/joomla-browser#133

@brianteeman
Copy link
Contributor Author

there are 17 changes in total to make and it will take you about 5 minutes

@Bakual
Copy link
Contributor

Bakual commented May 24, 2017

We should change this later, not on minor versions.

This change is absolutely fine for patch versions (it's not even a minor version).
If the codeception tests fail, they need to be fixed.

On a sidenote: As long as those tests don't run automatically on Pull Requests, we cant make decisions based on those tests anyway.

@810
Copy link
Contributor

810 commented May 24, 2017

But our componens/modules/plugins/templates are running on the pr's, so we are having failed tests now.
We need to improve the joomla browser and our own tests on each rep.

I will improve it, but it was better todo on J3.8, now we have b/c, some 3rth parties will have this issue.

@mbabker
Copy link
Contributor

mbabker commented May 24, 2017

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.

@brianteeman
Copy link
Contributor Author

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

@Bakual
Copy link
Contributor

Bakual commented May 24, 2017

now we have b/c, some 3rth parties will have this issue.

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.

@mbabker
Copy link
Contributor

mbabker commented May 24, 2017

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.

@brianteeman
Copy link
Contributor Author

brianteeman commented May 24, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Language Change This is for Translators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants