rnx-kit
rnx-kit copied to clipboard
List of improvements for incubating package `patch-rnmacos`
This issue wants to track a series of improvements that can be done for the draft package @rnx-kit/patch-rnmacos
to get it to be is a better shape (and potentially graduate from draft into something that we'd like to share more broadly).
Here's the list:
- [ ] add proper testing. Currently, there is none.
- [ ] Some modules names are camelCase and some are snake_case. Make it consistent within the package (either one is fine).
- [ ] the tool currently defaults to Windows values: it should be able to figure out what platform it is on and choose an appropriate set of executables.
- [ ] change the
--confirm true
param into--write
without a value. - [ ] change
program.version("0.0.1");
incli.ts
to pick up the version from thepackage.json
's fieldversion
- [ ] move the default inclusion and exclusion lists out of code and into a json or text file.
- [ ] in
diffRepos.ts
, the code between the onHit and onMiss callbacks is nearly identical. Lift it out and share - [ ] remove commented out code if deemed useless
- [ ] replace
packages/draft-patch-rnmacos/src/file_compare.ts
withhasha.fromFileSync()
(from https://github.com/sindresorhus/hasha). It does the same thing, it's maintained by someone else, and it uses worker threads for faster perf. - [ ] make exclusion list be pre-processed to exclude ./ or .\ once, ahead of time. See for example ~L114 in
packages/draft-patch-rnmacos/src/fs_utils.ts
- [ ] Git can generate and apply file-level patches. Why not use that, vs writing our own impl of parsing and applying patch files? (see
patch/apply.ts
as an example of something that could be replaced potentially) - [ ] a few more improvements for
patch/apply.ts
:- [ ] change logic on L30 - from comment: "What about a check to see if the target exists? If it does, that seems like a problem.... by default, moveSync fails if the target exists already."
- [ ] comment: The dryRun checks seem valuable in the non-dryrun case. Consider running them all the time, and then only doing the destructive ops when !dryRun.
- [ ] L63 - from comment: "What about other mode changes, like read and write?"
- [ ] logic in L176->L197 for
packages/draft-patch-rnmacos/src/diffRepos.ts
should be revised: If something is present in both inclusionList as well as exclusionList, you should probably fail the whole run. - [ ] add a new command
update-patches
, to attempt fixing a patch after the codebase has changed. See an attempt at implementation here: https://github.com/microsoft/react-native-macos/pull/864
Feel free to pick up any one of these bullet points separately - simply leave a comment here to let everyone know that you are working on it.
most of the items listed were originally reported here https://github.com/microsoft/rnx-kit/pull/1135
- [ ] patch tool has a number of default app paths in it. these should be replaced with parameterized values.
- [ ] patch tool makes assumptions about running on Windows. it needs to handle cases where it is run on MacOS or Ubuntu (CI loops).