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

Add Blazor Hybrid to no-loc metadata #25064

Merged
merged 3 commits into from
Feb 18, 2022
Merged

Conversation

guardrex
Copy link
Collaborator

Addresses #24956

We want to make sure that Blazor Hybrid isn't localized, just like Blazor WebAssembly and Blazor Server.

IIRC, we're adding the global changes (via find-'n-replace) to the front of the list, while we'd add individual terms/phrases for specific topics to the end.

@guardrex guardrex mentioned this pull request Feb 18, 2022
25 tasks
@guardrex
Copy link
Collaborator Author

@serpent5 ... The failures are unrelated to the PR, so we're not going to hold this up on those.

@serpent5
Copy link
Contributor

serpent5 commented Feb 18, 2022

I've just run some regex magic to confirm that the "local" tokens are added to the end of the list, so this looks good. We actually have a few no-locs that aren't consistent with our global list in the repo, but we can revisit that.

I'd rather see "Blazor Hybrid" after "Blazor WebAssembly" in the global list, but that's just really picky and perhaps not practical.

Anyway, I'm happy for you to go ahead with this as it is. I will take on a follow-up PR to fix the warnings highlighted here.

@guardrex
Copy link
Collaborator Author

I'd rather see "Blazor Hybrid" after "Blazor WebAssembly" in the global list

...... ah, BUT ... what you just said ...

have a few no-locs that aren't consistent

... is exactly why when there are hundreds ... like 600+ of these ... adding to the front the safest approach. It's sure-fire going to work every time. The inconsistencies were bound to show up over time, so we just said that we'd place new global ones on the front .and per-doc ones on the end. It should work if the 😈 stay out of our no-loc lists; specifically, duplicates could be a potential problem. True, the order isn't reasonable, but all of the global entries will go away one day, per the following ............

What we really need is a global list in the repo config, and I think that was something Jason said they were thinking about or planning. It's more of an internal docs thing, so I'll leave it to the team if someone wants to ping Khoi Pham and ask about that.

@guardrex
Copy link
Collaborator Author

I won't hold u to account for this one. I'll go ahead without review, and they can RETIRE ME if they want 🏖️🇧🇧🍹😄 if things go sideways.

@guardrex guardrex removed the request for review from serpent5 February 18, 2022 12:52
@guardrex
Copy link
Collaborator Author

guardrex commented Feb 18, 2022

🐀🍔 Roasted rat burgers! I can't override those failures.

I'll fix them and then merge this.

@serpent5
Copy link
Contributor

I think I can override them and do the merge. Shall I?

@serpent5
Copy link
Contributor

Just FYI, I count 30 articles that aren't using the complete "global" list. Some of those have extras, and some have missing tokens.

@guardrex
Copy link
Collaborator Author

I think I can override them and do the merge. Shall I?

That's ok ... I'll fix them. We need them fixed anyway, so I may as well. I'm working on them right now.

that aren't using the complete "global" list

That's up to the team to address. AFAIK, the Blazor ones are consistent.

@serpent5
Copy link
Contributor

AFAIK, the Blazor ones are consistent.

Yup, that's what I'm seeing too.

I'm more than happy to approve this PR. The comments I made were just higher-level. The approach you've got here makes sense to me. I did try and submit an approval, but it keeps hanging my browser. 🐌

@guardrex guardrex merged commit 5f1743e into main Feb 18, 2022
@guardrex guardrex deleted the guardrex/blazor-hybrid-noloc branch February 18, 2022 13:17
@guardrex
Copy link
Collaborator Author

Ok ... that lick'd 'um! 🐮👅

ONE more thing to do before the 3-day weekend! Woot!

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

Successfully merging this pull request may close these issues.

2 participants