Extensions Strings Organization #3575
Conversation
…ith an extension comment.
|
Since we're past string freeze and this could complicate any last-minute translation changes, we'll have to wait until Sprint 25 before merging. |
|
Yes, I know. Just wanted to do it in case I would forget later. |
ghost
assigned 
|
Assigning to myself |
|
@TomMalbran Changes look good but we have a small conflict. Will have to merge with master first. Also, I wouldn't like to merge this without propagating this same work to the rest of language files. I'm afraid it will add a lot of noise if we don't do that and I'll prefer to keep them as much in sync as we can. Would you have the time to look into it? Otherwise I can take care of that. |
|
@jbalsas I fixed the merge issues and went through all the other nls files doing the same organization with the strings that were translated, and as you know, a lot are missing so I moved what I could. |
|
@TomMalbran Wow... that's a lot of work you did! Thanks! Reviewing |
|
@TomMalbran This looks great! Thanks a lot for the work! As in the other commit, 70c7210 seems off. My guess is that you pulled from master onto your branch which created that commit. If that's the case, it could be better to do I guess it's ok to merge, but waiting on @njx to advice in wether it's safe since I'm not a git-guru ;) |
| "COLOR_EDITOR_USED_COLOR_TIP_PLURAL" : "{0} ({1} 次)" | ||
| "COLOR_EDITOR_USED_COLOR_TIP_PLURAL" : "{0} ({1} 次)", | ||
|
|
||
| "CMD_JSLINT" : "启用JSLint", |
There was a problem hiding this comment.
We're missing here the //extensions/default/JSLint but we can certainly merge without that ;)
Edit: moved the annotation one line back to get context
|
@jbalsas Did the same here as in #3336. Fetched the latest commits and merged them in the branch. But since there was a merge issue, I had to fix it and commit it, so it doesn't show up as other merges. But I added the fixes you mentioned inside the merge and didn't realized that checking the commit alone would be a mess, but the diff GitHub makes is ok. Next time I can split the fixes with the merge fixes. |
|
It's probably worth actually rebasing instead of just merging if you can. Normally we're not a stickler for rebasing, but for pull requests that start with old commits it's probably a good idea. (I know it's our fault that we're not pulling these things in quickly enough...sorry!) |
|
This one was partially my fault too, since I submitted it past string freeze. But in this request the change at 70c7210 isn't that big, considering the pictures, the Getting Started content and the Notice update, and the big change I did on every language. |
|
I am not great at GitHub yet and every time I tried rebasing I made a mess for some reason, so if is possible to merge this one like this, that would be great. This one doesn't seem to have so many changes in 70c7210 as the other one does, and there are lots of changes I made to all the languages. |
|
Hey @TomMalbran, sorry to get back at you so late. I've been playing around with this and it seems safe to merge as is. I've merged locally and everything seems to be working fine, so I don't think we'd have any problems. However, since I'll be flying out tomorrow and the guys will be at MAX for the next couple of days, I'll wait to merge this until after that, just in case the repo gets messed up somehow. Thanks a lot for looking into this! |
|
@TomMalbran the main thing about rebasing is that you're changing history. You don't want to change history on a branch that is out there in the public and that people may have copies of. If you wanted to rebase a change that has been pushed publicly, you could create a new branch, rebase that branch against master, squash commits as needed, etc. Then push that branch to master. In theory, that shouldn't break anyone's repos because that commit to master is the only thing that appears publicly and it would just appear as a new commit. |
|
(BTW, I added that comment just to hopefully make a helpful point about rebasing. I do not want to imply that you need to rebase this particular change, which I haven't looked at.) |
|
I took a look at rebasing this and it would be a huge pain since there were a number of conflicts along the way. So I think we should just merge this as-is without rebasing. (However, it looks like it will need another merge with master.) |
|
I just merged with master and fixed the merge issues on the first commit. So that one will show many changes, but is basically a merge. I then moved the new locale strings to the extension section. I see that there was a Travis fail, but the problem was at the submodule update. Should I add a new commit to make Travis run again? |
|
Sure, that sounds fine. Thanks. |
|
Looks good. Merging. @TomMalbran Thanks a lot for working on this! Sorry I couldn't get to merge this sooner :S |
Extensions Strings Organization
I noticed that the command strings for a few extensions and other language switcher strings where mixed with the core strings, so I moved them to the bottom of the file.