Skip to content

ENG-2059 Refactor existing Roam to CrossApp converter into its own file.#1243

Open
maparent wants to merge 1 commit into
mainfrom
eng-2059-refactor-existing-roam-to-crossapp-converter-into-its-own
Open

ENG-2059 Refactor existing Roam to CrossApp converter into its own file.#1243
maparent wants to merge 1 commit into
mainfrom
eng-2059-refactor-existing-roam-to-crossapp-converter-into-its-own

Conversation

@maparent

@maparent maparent commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

@linear-code

linear-code Bot commented Jul 22, 2026

Copy link
Copy Markdown

ENG-2059

@supabase

supabase Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
discourse-graph Skipped Skipped Jul 22, 2026 10:02pm

Request Review

text: string;
node_type_id: string;
node_title?: string;
fullText?: string;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Rather than add a parameter to the converter, I thought I could add the full text here, as it feels natural for the type.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +5 to +7
export const RoamFullContentNodeToCrossApp = (
node: RoamFullContentNode,
): CrossAppNode => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 New converter function name does not follow the project's naming convention

The newly added converter function is named starting with an uppercase letter (RoamFullContentNodeToCrossApp at apps/roam/src/utils/roamToCrossAppConverters.ts:5) instead of camelCase as required for functions by the project style guide, so it breaks the repository's mandated naming convention.

Impact: Code diverges from the required style, hurting consistency and maintainability.

Naming convention rule

AGENTS.md mandates "camelCase for variables and functions" and "PascalCase for components and types". RoamFullContentNodeToCrossApp is a plain utility function (not a component or type), so it should be roamFullContentNodeToCrossApp. The import at apps/roam/src/utils/convertRoamNodeToFullContent.ts:9 and usage at line 63 would need to be updated accordingly.

Prompt for agents
The function RoamFullContentNodeToCrossApp in apps/roam/src/utils/roamToCrossAppConverters.ts is a utility function but uses PascalCase, violating the AGENTS.md rule that functions must be camelCase. Rename it to roamFullContentNodeToCrossApp and update its import and usage in apps/roam/src/utils/convertRoamNodeToFullContent.ts (import on line 9, call on line 63).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@maparent
maparent force-pushed the eng-2059-refactor-existing-roam-to-crossapp-converter-into-its-own branch from 82c5654 to e02622c Compare July 22, 2026 22:00
@maparent
maparent force-pushed the eng-2059-refactor-existing-roam-to-crossapp-converter-into-its-own branch from e02622c to fc9ebdf Compare July 22, 2026 22:02
@maparent
maparent requested review from mdroidian and sid597 July 23, 2026 01:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant