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

Javascript to JavaScript #10552

Closed
wants to merge 4 commits into from
Closed

Javascript to JavaScript #10552

wants to merge 4 commits into from

Conversation

lipis
Copy link
Contributor

@lipis lipis commented Sep 24, 2016

@lipis
Copy link
Contributor Author

lipis commented Sep 24, 2016

There are plenty of cases that it's used as javascript in some comments as well.. but that's quite more challenging to detect without ruining everything :D

@lipis
Copy link
Contributor Author

lipis commented Sep 25, 2016

Not sure about the tests :/ Checked again if I touched anything else than all the Javascript entries.. but it seems nothing else was changed..

@sgiehl
Copy link
Member

sgiehl commented Sep 25, 2016

Thanks for your PR. Unfortunately we can't merge this PR for various reasons.

  • You've changed many translations files, but our translations are managed using transifex. So all changes would be overwritten with the next sync
  • composer.lock shouldn't be changed manually
  • Also you've changed some files of third party libs that are included in the tests. We should leave those files untouched

@lipis
Copy link
Contributor Author

lipis commented Sep 25, 2016

Just be more specific.. I don't need to know all the reasons.. @mattab said exclude libs/ and that's what I did..

Should I ignore all the lang folder.. maybe leaving the en.json? Which one is your master file?

@sgiehl
Copy link
Member

sgiehl commented Sep 25, 2016

All en.json files are master files. Those can be changed the other ones shouldn't.

@lipis
Copy link
Contributor Author

lipis commented Sep 25, 2016

and which folders I should exclude from the test folder?

@sgiehl
Copy link
Member

sgiehl commented Sep 25, 2016

tests/javascript/assets
tests/javascript/frameworks
tests/lib

@mattab mattab added this to the 3.0.0-b2 milestone Sep 30, 2016
@mattab mattab added the Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. label Sep 30, 2016
@mattab
Copy link
Member

mattab commented Sep 30, 2016

@lipis would be great if you could exclude tests/javascript/frameworks/ and tests/libs from the changes (as better not to update third party libs to not affect git history).

The other changes in non-english translation files, I wouldn't mind merging anyway, as they will just get overwritten later and it does not matter. Maybe we could even separately do a mass replace of javascript in Transifex @sgiehl ?

Thanks for the PR!

@sgiehl
Copy link
Member

sgiehl commented Oct 2, 2016

mass replacement isn't possible. If we change all occurrences of 'Javascript' in all en.json, all those strings will be invalidated on transifex and someone needs to retranslate them. If we don't want to loose those translations as it's a minor change, guess I would need to go through all those translations on transifex and retranslate them, which is quite time consuming (but wouldn't be the first time)

@mattab
Copy link
Member

mattab commented Oct 2, 2016

As there were only 2 strings in english changed, I would say it's fine to invalidate those two translations across all languages.

@lipis
Copy link
Contributor Author

lipis commented Oct 2, 2016

Or upload the sources as well after downloading them and maybe changing them again :)

@sgiehl
Copy link
Member

sgiehl commented Oct 2, 2016

That would be the same effort I guess and would have that risk that translations would be overwritten if someone is translating something between download and upload.

@lipis
Copy link
Contributor Author

lipis commented Oct 2, 2016

Updated!

@mattab
Copy link
Member

mattab commented Oct 2, 2016

If the tests will pass LGTM

@mattab mattab self-assigned this Oct 3, 2016
@mattab mattab modified the milestones: 3.0.0-b2, 3.0.0-b3 Oct 30, 2016
@lipis lipis closed this Nov 6, 2016
@lipis lipis deleted the JavaScript branch November 6, 2016 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants