[GitHub] [daffodil-vscode] stevedlawrence commented on a diff in pull request #620: Launch wizard updates and rename data editor vars

["stevedlawrence (via GitHub)" <gi...@apache.org>]

stevedlawrence commented on code in PR #620:
URL: https://github.com/apache/daffodil-vscode/pull/620#discussion_r1184966125


##########
src/utils.ts:
##########
@@ -147,6 +150,15 @@ export function getConfig(
     daffodilDebugClasspath: daffodilDebugClasspath
       ? daffodilDebugClasspath
       : defaultConf.get('daffodilDebugClasspath', ''),
+    dataEditorOmegaEditPort: dataEditorOmegaEditPort
+      ? dataEditorOmegaEditPort
+      : defaultConf.get('dataEditor.omegaEditPort', 9000),

Review Comment:
   I don't think I have a strong opinion one way or the other in general--there's good arguments for both sides. It's probably easier to detect typos or configuration name changes if you don't do defaults, so I'd maybe lean towards no default.
   
   That said, in this particular case it looks like all uses of `defaultConf.get(...)` provide a default value, so at least in this PR I suggest we stay consistent with that and provide a default value.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org