Content: Remove `notranslate` on <pre> elements

[Elchi3]

Authors don't need to add the notranslate class says @peterbe:

Just FYI, you don't need to insert the notranslate into \

 tags. It gets automatically inserted in build-time.

  https://github.com/mdn/content/pull/1190#discussion_r555781107

Filing this issue since I often copy from existing sources where this is present, so we could probably remove it everywhere as a clean-up.

[schalkneethling]

Filing this issue since I often copy from existing sources where this is present, so we could probably remove it everywhere as a clean-up.

Thanks for filing. I thought it might have been something we do to ensure all these elements have it set but, "best practice" is to add it when you author content. I guess it is one less thing to worry about then :)

[peterbe]

I thought it might have been something we do to ensure all these elements have it set but

It always gets injected on every <pre> tag automatically at build-time.

[Elchi3]

So, in many PRs, I now see authors removing notranslate. It adds a lot of noise to the PR reviews. (cc @hamishwillee).

Is there a way to batch-remove this once and for all? @nschonni?

[nschonni]

Aside I'm not sure it makes sense to globally remove notranslate on all PREs. Maybe i'm misunderstanding the usage, but I would think it's for signalling that some blocks do (not) need translation when copied for other langauages.
EX:

Doesn't need translation because it is a pure code

var count = 0;
for(var i = 0; i<40; i++) {
   count++;
}

Might need a translation because it uses English strings:

// Some explanatory comment that might have meaning
for(var i = 0; i<40; i++) {
   console.log("Hello world"); // An English phrase
}

[Elchi3]

hmm, good point @nschonni.

I don't understand why https://github.com/mdn/content/pull/1124 made sure we have notranslate on all pre elements. @schalkneethling?

[peterbe]

There is little doubt that every <pre> tag should have the notranslate class. But it's always automatically injected in build-time.

If you see a PR whose only update is to remove the notranslate, close the PR. But that's "painful" because you have to explain the author why it's a useless PR (so their feelings aren't hurt).

I (or @nschonni ) can write a monster batch thing that removes it in batches of 1,000 pages if that removes the traffic of these PRs. Is it worth doing do you think @Elchi3 ? It might be worth doing if these PRs come up a lot.

[schalkneethling]

hmm, good point @nschonni.

I don't understand why #1124 made sure we have notranslate on all pre elements. @schalkneethling?

So, the reason I added notranslate manually, was because I misunderstood the intent of the scripting we do. My understanding was that we have the scripting in place until the content has made the required changes and we will then drop the scripting.

Turns out that is not the case so, that manual work was largely wasted effort. Ah well 😄

[Elchi3]

Thanks for elaborating @schalkneethling!

The only annoying thing for me is when PR authors "taint" the review with extra changes like removing notranslate all over a document.
I haven't seen PRs that have been opened to only remove notranslate, but yes, these should probably be closed and be pointed to this issue then.

[hamishwillee]

So was the resolution that we batch fix these or that I stop fixing them as I go along (because while they are unnecessary they also do no harm)?

I vote for batch fix :-)

[peterbe]

I vote we do nothing. If you happen to be in there touching the near-by lines, then go ahead and remove it.
If you see someone making a tiny PR just to remove the notranslate, close it.

There are over 18k occurrences of notranslate in the en-US content across 6,853 files.
So a batch fix would be quite disruptive and would probably require to be broken up into multiple PRs which each could cause a bunch of confusing merge conflicts.

[Elchi3]

I agree the changes do no harm, but they add noise to the review diff. Also, if one wants to revert a PR, then you also revert these unrelated changes, and so on. It just feels unclean to me.

I vote to batch fix, too, also given Schalk initially batch introduced these, so looks like that is in fact possible.

[Ryuno-Ki]

We had other occurences of batch fixes in chunks (e.g. limited to a subtree), so it's better to get to a clean state quickly.

[peterbe]

I agree the changes do no harm, but they add noise to the review diff. Also, if one wants to revert a PR, then you also revert these unrelated changes, and so on. It just feels unclean to me.

I vote to batch fix, too, also given Schalk initially batch introduced these, so looks like that is in fact possible.

Let's do it then. I'll get it started.

[peterbe]

https://github.com/mdn/content/pulls?q=is%3Apr+is%3Aclosed+notranslate+author%3Apeterbe

[Elchi3]

Thanks for cleaning this up, Peter! :)

[hamishwillee]

Yes, thanks very much.