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
KeyError
ifkind
does not exist indevfile_parser.rb
? - Question: Are we fine with failing
KeyError
if some of the keys don't exist indevfile_parser.rb
?- Idea: Validate the pod spec using
JSONSchemer
upfront 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: workspace
and 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.rb
to highlight the fact that git config is written to local repository. - Determine whether to handle
KeyError
more gracefully indevfile_parser.rb
and consider usingJSONSchemer
upfront for validation. - Consider updating parameter passing in
remote_development_shared_contexts.rb
to either infer attributes fromworkspace
or use a factory with good defaults.
- Question (non-blocking) Are the trailing semicolons needed in
Edited by 🤖 GitLab Bot 🤖