Skip to content

Conversation

@jiridanek
Copy link
Member

@jiridanek jiridanek commented Oct 31, 2025

https://redhat-internal.slack.com/archives/C0961HQ858Q/p1761933985263029?thread_ts=1761328992.287889&cid=C0961HQ858Q

Description

I fully admit this is less than half-baked at this point, but I believe the core idea of committing to use Python, Go, and TypeScript is already something we can discuss. The decision has been originally made back in Adriel's team, but we never wrote it down back then, besides in that Jira comment.

How Has This Been Tested?

Some time ago we abandoned skodjob/odh-e2e because it was written in Java and we decided not to do Java. That's the Jira I linked above.

Given the teams experience and preferences spreadsheet, Python, Go, and TypeScript seem reasonable choices.

Self checklist (all need to be checked):

  • Ensure that you have run make test (gmake on macOS) before asking for review
  • Changes to everything except Dockerfile.konflux files should be done in odh/notebooks and automatically synced to rhds/notebooks. For Konflux-specific changes, modify Dockerfile.konflux files directly in rhds/notebooks as these require special attention in the downstream repository and flow to the upcoming RHOAI release.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • Documentation
    • Added an official architecture decision documenting a preferred language order (Python, Go, TypeScript) over Bash for new development. Includes rationale, consequences, migration guidance, type-checking and editor support recommendations, and language-specific guidance to steer future projects and tooling choices. No functional or code changes were introduced.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci openshift-ci bot requested review from atheo89 and dibryant October 31, 2025 17:57
@openshift-ci openshift-ci bot added the size/m label Oct 31, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Walkthrough

Adds a new architecture decision record preferring Python, then Go, then TypeScript (in that order) over Bash for new development; includes context, decision, consequences, migration guidance, and language-specific notes. No code or API changes.

Changes

Cohort / File(s) Change Summary
Architecture Decision Document
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md
Added ADR that sets a language preference policy (Python > Go > TypeScript) over Bash, with rationale, migration guidance, notes on typechecking/editor support, and consequences. No functional or API changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review focus: consistency with existing ADR format, clarity of migration guidance, and correctness of language-specific recommendations (typechecking/editor notes).

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the main points but is missing testing instructions and manual testing verification checkboxes that are required by the template's merge criteria. Complete the merge criteria checklist by checking the remaining boxes or explaining why testing instructions and manual verification are not applicable for documentation-only changes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: documenting a language preference decision for Python, Go, and TypeScript in an ADR document.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel label Oct 31, 2025
@openshift-ci openshift-ci bot added size/m and removed size/m labels Oct 31, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (2)

1-1: Standardize capitalization to TypeScript (official brand).

The document uses both "Typescript" (lines 1, 19) and "TypeScript" (lines 42+). Standardize to the official brand spelling "TypeScript" throughout for consistency and correctness.

-# 3. Prefer Python, Go, and Typescript (in this order)
+# 3. Prefer Python, Go, and TypeScript (in this order)
-The team has agreed to use Python, Go, and Typescript, in this order.
+The team has agreed to use Python, Go, and TypeScript, in this order.

Also applies to: 19-19, 42-42


15-15: Simplify wordiness: replace "in preference to" with "over".

LanguageTool flags both instances of "in preference to" (lines 15 and 48) as potentially wordy. Consider using the more concise "over" instead.

-Use these languages in preference to Bash.
+Use these languages over Bash.
-We will consider using full-stack TypeScript in preference to having either a Python or Go backend to a TypeScript frontend.
+We will consider using full-stack TypeScript over Python or Go backends with a TypeScript frontend.

Also applies to: 48-48

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 731c89f and f16f705.

📒 Files selected for processing (1)
  • docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md

[style] ~15-~15: ‘in preference to’ might be wordy. Consider a shorter alternative.
Context: ...degree) TypeScript. Use these languages in preference to Bash. ## Decision The team has agreed...

(EN_WORDINESS_PREMIUM_IN_PREFERENCE_TO)


[style] ~48-~48: ‘in preference to’ might be wordy. Consider a shorter alternative.
Context: ...ll consider using full-stack TypeScript in preference to having either a Python or Go backend to...

(EN_WORDINESS_PREMIUM_IN_PREFERENCE_TO)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: pytest-tests
  • GitHub Check: code-static-analysis
🔇 Additional comments (1)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1)

9-54: Document quality and structure are strong.

The ADR follows proper format with clear sections (Context, Decision, Consequences) and appropriate rationale for the language choices. The language-specific guidance (lines 26–48) provides helpful caveats and context—e.g., acknowledging Python's verbosity trade-off, Go's performance benefits, and TypeScript's web-native strengths. The narrow Bash scope (line 24) is pragmatic and clear.

Note: The PR checklist shows testing and manual verification unchecked; this is reasonable for a documentation-only change.

@openshift-ci openshift-ci bot added size/m and removed size/m labels Oct 31, 2025
@jiridanek jiridanek marked this pull request as draft October 31, 2025 18:05
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 5, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jiridanek for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jiridanek jiridanek marked this pull request as ready for review December 5, 2025 19:15
@openshift-ci openshift-ci bot requested a review from ysok December 5, 2025 19:16
@openshift-ci openshift-ci bot added size/l and removed size/l labels Dec 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (4)

15-15: Simplify wording for clarity.

The phrase "in preference to" is slightly wordy. Consider: "Use these languages instead of Bash."

Apply this diff:

-Use these languages in preference to Bash.
+Use these languages instead of Bash.

71-71: Add hyphen for compound adjective.

"Second class citizen" should be hyphenated when used as a compound adjective.

Apply this diff:

-<span style="color:green;">⊕</span> Playwright has best support for TypeScript, other language bindings are a bit of a second class citizen.
+<span style="color:green;">⊕</span> Playwright has best support for TypeScript, other language bindings are a bit of a second-class citizen.

92-92: Strengthen verb choice.

The phrase "seems to be more vibrant and dynamic" uses informal language. Consider a more definitive alternative.

Apply this diff:

-Pyrefly is more complete, whereas ty seems to be more vibrant and dynamic.
+Pyrefly is more complete, whereas ty is more vibrant and dynamic.

98-98: Remove informal qualifier.

The phrase "pretty much given" is informal. Strengthen by removing the qualifier.

Apply this diff:

-VSCode support is pretty much given these days.
+VSCode support is standard these days.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f16f705 and c089172.

📒 Files selected for processing (1)
  • docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md

[style] ~15-~15: ‘in preference to’ might be wordy. Consider a shorter alternative.
Context: ...degree) TypeScript. Use these languages in preference to Bash. ## Decision The team has agreed...

(EN_WORDINESS_PREMIUM_IN_PREFERENCE_TO)


[grammar] ~71-~71: Use a hyphen to join words.
Context: ... language bindings are a bit of a second class citizen. <span style="color: gray...

(QB_NEW_EN_HYPHEN)


[style] ~92-~92: Consider a more formal alternative to strengthen your wording.
Context: .... Pyrefly is more complete, whereas ty seems to be more vibrant and dynamic. Either ...

(SEEM_APPEAR)


[style] ~98-~98: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...Python type checkers VSCode support is pretty much given these days. In IntelliJ world, [...

(PRETTY_MUCH)

🔇 Additional comments (2)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (2)

1-122: Well-structured ADR with clear decision and rationale.

The document effectively communicates the language preference policy, provides substantive context for the decision, and offers helpful technical context on each language choice. The structure and depth are appropriate for an ADR.

Minor suggestions below.


7-7: Clarify the document status.

The status is marked as "Proposed," but the PR context suggests this decision has been previously agreed upon by the team (per RHOAIENG-14780 reference in PR objectives). Consider updating to "Accepted" to reflect that this is documenting an existing decision, or clarify in the Context why it remains in "Proposed" state awaiting formal approval.

@openshift-ci openshift-ci bot removed the size/l label Dec 5, 2025
@openshift-ci openshift-ci bot added size/l and removed size/l labels Dec 5, 2025
@jiridanek jiridanek added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1)

100-107: Add guidance to check current documentation for editor tool versions.

The specific version requirement "2025.3+" for IntelliJ/PyCharm will become stale as new versions release. Consider adding a note recommending developers verify current tool compatibility.

Apply this diff to add staleness safeguard:

 In IntelliJ world,
-[version 2025.3+ is required for Python tools](https://www.jetbrains.com/help/pycharm/2025.3/lsp-tools.html)
+[recent versions (2025.3+) are recommended for Python tools](https://www.jetbrains.com/help/pycharm/latest/lsp-tools.html)
 support, as previous versions don't yet have
 [LSP support built in](https://plugins.jetbrains.com/docs/intellij/language-server-protocol.html).
+
+Note: Check the [latest PyCharm documentation](https://www.jetbrains.com/help/pycharm/latest/) for current version requirements.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c089172 and bd6bd54.

📒 Files selected for processing (1)
  • docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md

[style] ~98-~98: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...Python type checkers VSCode support is pretty much a given these days. In IntelliJ world,...

(PRETTY_MUCH)

🔇 Additional comments (4)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (4)

1-8: ADR metadata structure is sound.

The document follows proper ADR conventions with clear title, date, and status.


9-25: Context and decision are well-articulated.

The rationale is grounded in real team experience and provides clear guidance on language ordering and Bash thresholds.


108-122: Consequences section effectively articulates benefits.

The section clearly outlines improved maintainability, testing, and code reuse. The closing on low-ceremony code balances the emphasis on fearless refactoring.


85-87: Pyrefly and ty are legitimate emerging Python type checkers—no correction needed.

Contrary to the original concern, "Pyrefly" (line 85) and "ty" (line 87) are both real, recently emerged Rust-based Python type checkers created by Meta and Astral respectively. They are actively developed tools designed for ultra-fast incremental type checking, making them valid modern alternatives alongside Pyright and basedpyright. All three tools listed are correct and current as of 2024–2025.

Likely an incorrect or invalid review comment.


### Editor support for Python type checkers

VSCode support is pretty much a given these days.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Replace informal phrase "pretty much" for clarity.

The static analysis tool flagged this phrase as informal. For an ADR, consider stronger wording.

Apply this diff:

-VSCode support is pretty much a given these days.
+VSCode support is standard across current versions.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
VSCode support is pretty much a given these days.
VSCode support is standard across current versions.
🧰 Tools
🪛 LanguageTool

[style] ~98-~98: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...Python type checkers VSCode support is pretty much a given these days. In IntelliJ world,...

(PRETTY_MUCH)

🤖 Prompt for AI Agents
In
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md
around line 98, replace the informal phrase "pretty much" in the sentence
"VSCode support is pretty much a given these days." with a stronger, more formal
phrasing such as "is effectively ubiquitous," "is now widely available," or "is
broadly supported" to improve clarity and tone for an ADR; update the sentence
to use one of these formal alternatives while preserving original meaning.

@openshift-ci openshift-ci bot added size/l and removed size/l labels Dec 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (1)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1)

98-98: Replace informal phrase "pretty much" with more formal language.

The phrase "pretty much" is informal for an ADR and weakens the clarity of the statement. Use stronger, more professional wording aligned with ADR conventions.

Apply this diff:

-VSCode support is pretty much a given these days.
+VSCode support is now widely available across current versions.
🧹 Nitpick comments (1)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1)

100-106: Consider adding a maintenance/review date for tool version references.

Lines 101–106 reference specific versions of IntelliJ PyCharm (2025.3+) and plugin support. While helpful, these details can become stale. Consider adding a brief note about when this guidance was last validated or when it should be reviewed.

Example addition:

In IntelliJ world,
-[version 2025.3+ is required for Python tools](https://www.jetbrains.com/help/pycharm/2025.3/lsp-tools.html)
+[version 2025.3+ is required for Python tools](https://www.jetbrains.com/help/pycharm/2025.3/lsp-tools.html) (as of 2025-10)
support, as previous versions don't yet have
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bd6bd54 and a59774a.

📒 Files selected for processing (1)
  • docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md

[style] ~98-~98: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...Python type checkers VSCode support is pretty much a given these days. In IntelliJ world,...

(PRETTY_MUCH)

🔇 Additional comments (1)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1)

1-121: ADR structure is well-organized; document establishes clear language preference.

The decision record effectively documents the team's language preferences with solid rationale, balanced pro/con analysis for each language, and practical guidance on typechecking and editor support. The decision is clearly stated and actionable for the team.

@openshift-ci openshift-ci bot added size/l and removed size/l labels Dec 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1)

100-110: Replace informal phrase "pretty much" for greater formality.

Line 102 uses the phrase "pretty much," which weakens the formal tone appropriate for an ADR. Replace it with stronger, more definitive language.

Apply this diff:

-VSCode support is pretty much a given these days.
+VSCode support is broadly available today across current versions.

This maintains the intended meaning while strengthening the formality and clarity of the statement.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a59774a and 04678ce.

📒 Files selected for processing (1)
  • docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md

[style] ~102-~102: The phrase ‘pretty much’ can be informal. To strengthen your writing, consider removing it or replacing it with an adverb.
Context: ...Python type checkers VSCode support is pretty much a given these days. In IntelliJ world,...

(PRETTY_MUCH)

🔇 Additional comments (5)
docs/architecture/decisions/0003-prefer-python-go-and-typescript-in-this-order.md (5)

1-28: ADR structure and motivation are clear.

The document correctly establishes context (Bash limitations and team skills), decision (Python→Go→TypeScript preference), and rationale. The YouTube link correction from prior review is in place.


26-42: Bash section provides solid rationale.

The evidence linking Bash to software-engineering unfriendliness (Titus Winters reference, type system limitations, and critiques of error-handling patterns) grounds the decision. The mention of bats acknowledges practical constraints.


44-80: Language comparisons are balanced and substantive.

Each language section provides concrete pros and cons. The TypeScript phrasing ("We may consider") at line 77 appropriately signals optionality for full-stack deployments, and the use of the information symbol (🛈) clarifies this is exploratory guidance. The inclusion of Brad Fitzpatrick's Go talk adds credibility.


82-98: Typechecking strategy is pragmatic.

The section acknowledges that type checking is currently disabled and proposes a sensible incremental approach: repair types with Pyright, gain experience, then re-evaluate. Listing checker options and their trade-offs (Pyrefly's completeness vs. ty's vibrancy) shows awareness of the landscape.


112-125: Consequences clearly articulate expected benefits and trade-offs.

The section balances measurable outcomes (reduced duplication, sane import mechanism, fearless refactoring) with awareness of trade-offs (verbosity of simple scripts, risk of overengineering). The James Shore quote is motivational and reinforces the engineering discipline message.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 5, 2025

@jiridanek: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/images 04678ce link true /test images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel size/l tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant