Skip to content

chore: scaffold rne-rewrite target branch (#1255)#1256

Open
msluszniak wants to merge 1 commit into
rne-rewritefrom
@ms/rewrite-branch-setup
Open

chore: scaffold rne-rewrite target branch (#1255)#1256
msluszniak wants to merge 1 commit into
rne-rewritefrom
@ms/rewrite-branch-setup

Conversation

@msluszniak

@msluszniak msluszniak commented Jun 17, 2026

Copy link
Copy Markdown
Member

Description

Sets up the long-lived rne-rewrite branch that all RNE rewrite PRs will target (see #1208). Keeps the repo's yarn monorepo layout, establishes the core first, and leaves per-task functionality to follow as separate block PRs.

  • Removes apps/ and the legacy packages/react-native-executorch.
  • Stands up the new core package by porting the rnet-poc lower-level API: cpp/core (tensor/model/dtype/install/utils), src/core, and the native bridge. MyLibrnexecutorch rename throughout. OpenCV and task extensions are intentionally deferred to the per-task PRs.
  • CI: deletes the apps/llm native build workflows and the stale error-code verification step; runs CI on rne-rewrite.
  • Meta: scopes lint/typecheck/build to the core package; the *-resource-fetcher and -webrtc packages stay on disk but are excluded from CI until their surface is reworked.

Introduces a breaking change?

  • Yes
  • No

(Targets the rne-rewrite dev branch; the published package on main is untouched.)

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Other (chores, tests, code style improvements etc.)

Tested on

  • iOS
  • Android

Testing instructions

CI is TypeScript-only here (no native compile). From the repo root:

yarn install
yarn workspaces foreach --all --exclude react-native-executorch-bare-resource-fetcher --exclude react-native-executorch-expo-resource-fetcher --exclude react-native-executorch-webrtc --topological-dev run prepare
yarn typecheck
yarn lint

All four pass green. On-device native build is a follow-up.

Related issues

Closes #1255. Part of #1208.

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Additional notes

  • Native ExecuTorch third-party binaries are not committed (see packages/react-native-executorch/third-party/README.md). CI doesn't need them; provisioning them (ideally via the repo's on-demand artifact mechanism) is a follow-up. OpenCV is dropped since CV is a later block.
  • The common/phonemis submodule is retained (unchanged from main) for later use by the TTS block.
  • The error-code codegen system is parked (the POC has none); scripts/generate-errors.ts stays dormant for later reintroduction.
  • Root submodules (tokenizers-cpp, googletest) are left in place for likely reuse by the LLM block; they aren't checked out in CI.
  • react-native-worklets emits peer-dep warnings (@babel/core, @react-native/metro-config) — harmless app-integration peers, not needed to build the lib.

@msluszniak msluszniak self-assigned this Jun 17, 2026
@msluszniak msluszniak added refactoring chore PRs that are chores labels Jun 17, 2026
@msluszniak msluszniak force-pushed the @ms/rewrite-branch-setup branch 3 times, most recently from 74a6ffe to c627eab Compare June 17, 2026 12:21
@barhanc barhanc self-requested a review June 17, 2026 13:30
@barhanc

barhanc commented Jun 17, 2026

Copy link
Copy Markdown
Contributor
  • The docs/docs directory should also probably be nuked, because it will have to be rewritten.
  • Also, don't we want to set the clang formatter config in this PR, because right now I see the .cpp files are still formatted in the MSVC style as the PoC, but style: add clang-format config and reformat C++ sources #1230 is the style we want.
  • Is it possible to set-up branch protection rules similar to main on the rewrite development branch?

@msluszniak msluszniak force-pushed the @ms/rewrite-branch-setup branch from c627eab to 36f208a Compare June 17, 2026 15:56
@msluszniak

Copy link
Copy Markdown
Member Author

Thanks — addressed in the latest push:

1. clang-format ✅ Brought in .clang-format (and .clang-format-ignore) from #1230 and reformatted the ported C++ to the LLVM-based style — the PoC/MSVC formatting is gone. Note the .clang-format-ignore patterns (common/ada, common/pfft, common/runner, ErrorCodes.h) don't exist on this branch yet; I kept them verbatim for parity so it lines up when those land / when #1230 reaches main.

2. docs ✅ (partial — want your call) Nuked docs/docs. I left docs/versioned_docs/ (~7.7 MB), docs/versioned_sidebars/ and docs/versions.json in place since they're old released-version snapshots and you only called out docs/docs — but by the same "it'll be rewritten" logic they're obsolete here too. Want me to nuke those as well (and trim sidebars.js/docusaurus.config.js accordingly)? The docs build only runs on main, so it won't gate this branch either way.

3. Branch protection ⚠️ needs an admin I can't do this from here — the branch-protection API returns 404 for my token (no admin scope). A maintainer would need to mirror main's ruleset onto rne-rewrite via Settings → Branches (or the rulesets API). Happy to provide the exact ruleset JSON if useful.

Comment thread .clang-format

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.

Personally I would prefer something more similar to: { BasedOnStyle: LLVM, UseTab: Never, IndentWidth: 4, TabWidth: 4, AllowShortIfStatementsOnASingleLine: false, IndentCaseLabels: false, ColumnLimit: 0, AccessModifierOffset: -4, FixNamespaceComments: false }. It is basically the default VS Code one, but doesn't put the { on the new line. Additionally regarding the column limit option, I find that clang formatter often makes very awkward line breaks to satisfy the column limit that often are not very readable, and it would be better to set this to no column limit and let the developers handle line breaks for very long lines. What do you think?

@msluszniak msluszniak Jun 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, so I think we can get a union of these two setups and disable fixed column limit. How about that?

@barhanc

barhanc commented Jun 17, 2026

Copy link
Copy Markdown
Contributor
  • We should also replace skills/ with https://github.com/barhanc/rnet-poc/tree/main/.agents/skills (and let's put them under .agents/ because otherwise e.g. antigravity doesn't see them). The current skills tell only how to use the library, the PoC skills tell how to build/extend it.
  • I'm not sure what to do with the scripts/error.config.ts and scripts/generate-errors.ts. Do we need them and if so how to integrate them into the rewrite?
  • .clang-format-ignore can be deleted for now as it only specifies stale paths that are nonexistent in the rewrite.
  • We have to edit the categories in .eslintrc.js.
  • In .gitmodules we have the tokenizers-cpp entry, but don't we bundle our custom tokenizers inside ExecuTorch now as per Migrate to PyTorch tokenizers #781 ?
  • The README, RELEASE.md and generic docs stuff like sidebar, etc. will have to be changed in the future. This, we should probably put in some TODO, either in the repo itself or in the tracking issue, so that we don't forget about it. (Added to tracking issue list)

@msluszniak

Copy link
Copy Markdown
Member Author

We should also replace skills/ with https://github.com/barhanc/rnet-poc/tree/main/.agents/skills (and let's put them under .agents/ because otherwise e.g. antigravity doesn't see them). The current skills tell only how to use the library, the PoC skills tell how to build/extend it.

Agreed. I will change it.

I'm not sure what to do with the scripts/error.config.ts and scripts/generate-errors.ts. Do we need them and if so how to integrate them into the rewrite?

I think that ultimately we want to have some custom error types etc. So we probably want something like that, but we may think if we can add some improvements idk. I would add a separate point in aggregated issue for now, and we can eventually copy this solution in the end.

.clang-format-ignore can be deleted for now as it only specifies stale paths that are nonexistent in the rewrite.

Yeap, will delete.

In .gitmodules we have the tokenizers-cpp entry, but don't we bundle our custom tokenizers inside ExecuTorch now as per #781 ?

Yes, but there are kept in current main because it was treated as source of truth in native tests for our implementation of tokenizers. So I guess we can keep it for now and write similar tests based on them. But if you want I can delete them as well.

@msluszniak msluszniak force-pushed the @ms/rewrite-branch-setup branch from 36f208a to 9d6783f Compare June 17, 2026 21:23
Set up the base for all RNE rewrite PRs (see #1208).

- Remove apps/ and the legacy packages/react-native-executorch.
- Stand up the new core package by porting the rnet-poc lower-level API:
  cpp/core (tensor/model/dtype/install/utils), src/core, and the native
  bridge. MyLib -> rnexecutorch rename throughout. OpenCV and task
  extensions are deferred to the per-task block PRs.
- Native ExecuTorch third-party binaries are NOT committed; CI is
  TypeScript-only and does not compile native code. They are provisioned
  separately before an on-device build (see third-party/README.md);
  wiring the on-demand mechanism is a follow-up. The common/phonemis
  submodule is retained for later TTS use.
- Adopt the clang-format config from #1230 (.clang-format) and reformat
  the ported C++ sources to the LLVM-based style.
- Replace the usage-oriented skills/ with the PoC's build/extend skills
  under .agents/skills (discoverable by AI coding agents), aligned to the
  rnexecutorch naming.
- Remove docs/docs content (the documentation will be rewritten).
- CI: drop the apps/llm native build workflows and the stale error-code
  verification step; run CI on the rne-rewrite branch.
- GitHub meta: scope lint/typecheck/build to the core package and keep
  the resource-fetcher and webrtc packages on disk but excluded from CI
  until their surface is reworked.
@msluszniak msluszniak force-pushed the @ms/rewrite-branch-setup branch from 9d6783f to fc41ba4 Compare June 17, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore PRs that are chores refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants