Follow-up from "Add user's git configuration in workspace"
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
The following discussion from !119621 (merged) should be addressed:
-
@splattael started a discussion: (+2 comments) - Question (non-blocking) Are the trailing semicolons needed in
devfile_processor.rb? - Observation (non-blocking) The git config is written to local repository and it might be worth highlighting this in the extensive code comment above.
- Question: Are we fine to fail with
KeyErrorifkinddoes not exist indevfile_parser.rb? - Question: Are we fine with failing
KeyErrorif some of the keys don't exist indevfile_parser.rb?- Idea: Validate the pod spec using
JSONSchemerupfront instead of usingfetch.
- Idea: Validate the pod spec using
- Observation (non-blocking) We might need to reconsider how the parameters are passed in
remote_development_shared_contexts.rb. Suggestions include passingworkspace: workspaceand allowing attributes to be inferred from there, or using a factory to construct those values and having good defaults.
Here's a quick summary of the code review:
- Remove trailing semicolons in
devfile_processor.rb. - Consider updating code comment in
devfile_processor.rbto highlight the fact that git config is written to local repository. - Determine whether to handle
KeyErrormore gracefully indevfile_parser.rband consider usingJSONSchemerupfront for validation. - Consider updating parameter passing in
remote_development_shared_contexts.rbto either infer attributes fromworkspaceor use a factory with good defaults.
- Question (non-blocking) Are the trailing semicolons needed in
Edited by 🤖 GitLab Bot 🤖