Skip to content

Conversation

@harshithajahagirdar
Copy link
Contributor

What It Does
Externalise strings

How to Test

Review Checklist
I certify that I have:

  • tested my changes
  • added/updated automated tests
  • updated the changelog
  • considered whether the docs need updating
  • followed the contribution guidelines

Additional Comments

@davenice
Copy link
Contributor

Hi Harshitha,

I know you're in progress with this and it's not a PR ready for review but I'm having a look now to see how things are going! Here are my thoughts and pointers...

  1. I like the way you've used keys that aren't the actual strings (e.g. cics.disableJVM.noServerSelected)

  2. I'm not keen on the ICommandProviderDialogs interface or the DisableJVMEndpointHandler because I think they add confusion. In the Zowe Explorer source they use vscode.l10n.t(...) directly. Now you've got things working 🎉 please can you move towards this approach?

The way I imagine this would look is that lines like this:
message: `Disabling JVM Endpoint '${resource.jvmendpoint}' (${i + 1} of ${nodes.length})`,
would turn into:
message: ` ${vscode.l10n.t("cics.disableJVMEndpoint.disabling")} '${resource.jvmendpoint}' (${i + 1} of ${nodes.length})`,

Actually it would be even better if your message was:
Disabling JVM Endpoint '{name}' {index} of {count}
This allows languages to translate word order correctly. You'd use syntax something like this (documented in the vscode.l10n.t doc)
message: ` ${vscode.l10n.t("cics.disableJVMEndpoint.disabling"), { name: ${resource.jvmendpoint}, index: ${i + 1}, count: ${nodes.length}}`, .... I haven't tried compiling this so it might not quite be the right syntax, but hopefully that gives you the direction.

This would then be less of a change from the existing code, without introducing lots of extra scaffold code.

We could then think about how we organise the strings within the codebase - all in one file, or in separate files?

@harshithajahagirdar harshithajahagirdar force-pushed the hershey-externalize-strings branch 4 times, most recently from aee6d2b to cc8ad8c Compare October 26, 2025 19:59
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.27%. Comparing base (e94e9ec) to head (991d3b7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/vsce/src/commands/setCICSRegionCommand.ts 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #458      +/-   ##
==========================================
+ Coverage   68.24%   68.27%   +0.02%     
==========================================
  Files         165      165              
  Lines        3883     3883              
  Branches      549      551       +2     
==========================================
+ Hits         2650     2651       +1     
+ Misses       1233     1232       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@harshithajahagirdar harshithajahagirdar force-pushed the hershey-externalize-strings branch from cc8ad8c to 200a0d5 Compare October 29, 2025 15:47
Signed-off-by: Harshitha J <[email protected]>
Signed-off-by: Harshitha J <[email protected]>
Signed-off-by: Harshitha J <[email protected]>
Signed-off-by: Harshitha J <[email protected]>
Signed-off-by: Harshitha J <[email protected]>
@harshithajahagirdar harshithajahagirdar force-pushed the hershey-externalize-strings branch from 7b03a42 to 991d3b7 Compare October 31, 2025 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

4 participants