[go: up one dir, main page]

Skip to content

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 if kind does not exist in devfile_parser.rb?
    • Question: Are we fine with failing KeyError if some of the keys don't exist in devfile_parser.rb?
      • Idea: Validate the pod spec using JSONSchemer upfront instead of using fetch.
    • Observation (non-blocking) We might need to reconsider how the parameters are passed in remote_development_shared_contexts.rb. Suggestions include passing workspace: 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 in devfile_parser.rb and consider using JSONSchemer upfront for validation.
    • Consider updating parameter passing in remote_development_shared_contexts.rb to either infer attributes from workspace or use a factory with good defaults.
Edited by 🤖 GitLab Bot 🤖