Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: comment diffs automatically #478

Open
wants to merge 137 commits into
base: main
Choose a base branch
from
Open

chore: comment diffs automatically #478

wants to merge 137 commits into from

Conversation

atlj
Copy link
Collaborator

@atlj atlj commented Oct 6, 2023

Summary:

This adds a new workflow that comments on the generated library diffs to your PR if you change a template.

How does it work?

This workflow automatically creates all possible libraries from your branch, then it does the same from the main branch. Lastly, it compares the generated libraries and comments on the diffs.

Notes:

  1. This PR also changes the install action to leverage yarn3's caching mechanisms.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

The diff

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

The diff
diff -r --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
8a9

// Hi mom!
diff -r --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
1a2
// Hi mom!
diff -r --no-ignore-file-name-case old-version/package.json new-version/package.json
68c68
< "react-native-builder-bob": "^0.20.0",


"react-native-builder-bob": "^0.23.1",

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

The diff

diff -r --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
8a9
> // Hi mom!
diff -r --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
1a2
>   // Hi mom!
diff -r --no-ignore-file-name-case old-version/package.json new-version/package.json
68c68
<     "react-native-builder-bob": "^0.20.0",
---
>     "react-native-builder-bob": "^0.23.1",

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR changes the output of create-react-native-library. You can find the diff of the change below:

diff -r -u --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
--- old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 17:59:32.000000000 +0000
+++ new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 17:59:26.000000000 +0000
@@ -6,6 +6,7 @@
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMethod;
 
+// Hi mom!
 public class ReactNativeTestModule extends ReactNativeTestSpec {
   public static final String NAME = "ReactNativeTest";
 
diff -r -u --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
--- old-version/babel.config.js	2023-10-06 17:59:32.000000000 +0000
+++ new-version/babel.config.js	2023-10-06 17:59:26.000000000 +0000
@@ -1,3 +1,4 @@
 module.exports = {
+  // Hi mom!
   presets: ['module:metro-react-native-babel-preset'],
 };
diff -r -u --no-ignore-file-name-case old-version/package.json new-version/package.json
--- old-version/package.json	2023-10-06 17:59:32.000000000 +0000
+++ new-version/package.json	2023-10-06 17:59:26.000000000 +0000
@@ -65,7 +65,7 @@
     "prettier": "^2.0.5",
     "react": "17.0.2",
     "react-native": "0.70.0",
-    "react-native-builder-bob": "^0.23.1",
+    "react-native-builder-bob": "^0.20.0",
     "release-it": "^15.0.0",
     "typescript": "^5.0.2"
   },

This diff is between 975d1f7 and the main branch.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

The commit with hash: 2827e62 has changed the output of create-react-native-library. You can find the diff of the change below:

diff -r -u --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
--- old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 17:59:53.000000000 +0000
+++ new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 17:59:43.000000000 +0000
@@ -6,6 +6,7 @@
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMethod;
 
+// Hi mom!
 public class ReactNativeTestModule extends ReactNativeTestSpec {
   public static final String NAME = "ReactNativeTest";
 
diff -r -u --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
--- old-version/babel.config.js	2023-10-06 17:59:53.000000000 +0000
+++ new-version/babel.config.js	2023-10-06 17:59:43.000000000 +0000
@@ -1,3 +1,4 @@
 module.exports = {
+  // Hi mom!
   presets: ['module:metro-react-native-babel-preset'],
 };

This diff is between this branch and the main branch.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR changes the output of create-react-native-library. You can find the diff of the change below:

diff -r -u --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
--- old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 18:06:19.000000000 +0000
+++ new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 18:06:13.000000000 +0000
@@ -6,6 +6,7 @@
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMethod;
 
+// Hi mom!
 public class ReactNativeTestModule extends ReactNativeTestSpec {
   public static final String NAME = "ReactNativeTest";
 
diff -r -u --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
--- old-version/babel.config.js	2023-10-06 18:06:19.000000000 +0000
+++ new-version/babel.config.js	2023-10-06 18:06:13.000000000 +0000
@@ -1,3 +1,4 @@
 module.exports = {
+  // Hi mom!
   presets: ['module:metro-react-native-babel-preset'],
 };

This diff is between 69b58ba9fced328a3e141a38c8c95377c1531baa and the main branch.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR changes the output of create-react-native-library. You can find the diff of the change below:

diff -r -u --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
--- old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 18:15:34.000000000 +0000
+++ new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 18:15:28.000000000 +0000
@@ -6,6 +6,7 @@
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMethod;
 
+// Hi mom!
 public class ReactNativeTestModule extends ReactNativeTestSpec {
   public static final String NAME = "ReactNativeTest";
 
diff -r -u --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
--- old-version/babel.config.js	2023-10-06 18:15:34.000000000 +0000
+++ new-version/babel.config.js	2023-10-06 18:15:28.000000000 +0000
@@ -1,3 +1,4 @@
 module.exports = {
+  // Hi mom!
   presets: ['module:metro-react-native-babel-preset'],
 };
diff -r -u --no-ignore-file-name-case old-version/package.json new-version/package.json
--- old-version/package.json	2023-10-06 18:15:34.000000000 +0000
+++ new-version/package.json	2023-10-06 18:15:28.000000000 +0000
@@ -65,7 +65,7 @@
     "prettier": "^2.0.5",
     "react": "17.0.2",
     "react-native": "0.70.0",
-    "react-native-builder-bob": "^0.23.1",
+    "react-native-builder-bob": "^0.20.0",
     "release-it": "^15.0.0",
     "typescript": "^5.0.2"
   },

This diff is between bf4e03bcae26d9357a3c285831d0ef17266d3f78 and the main branch.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR changes the output of create-react-native-library. You can find the diff of the change below:

diff -r -u --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
--- old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 18:22:10.000000000 +0000
+++ new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java	2023-10-06 18:22:04.000000000 +0000
@@ -6,6 +6,7 @@
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMethod;
 
+// Hi mom!
 public class ReactNativeTestModule extends ReactNativeTestSpec {
   public static final String NAME = "ReactNativeTest";
 
diff -r -u --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
--- old-version/babel.config.js	2023-10-06 18:22:10.000000000 +0000
+++ new-version/babel.config.js	2023-10-06 18:22:04.000000000 +0000
@@ -1,3 +1,4 @@
 module.exports = {
+  // Hi mom!
   presets: ['module:metro-react-native-babel-preset'],
 };
diff -r -u --no-ignore-file-name-case old-version/package.json new-version/package.json
--- old-version/package.json	2023-10-06 18:22:10.000000000 +0000
+++ new-version/package.json	2023-10-06 18:22:04.000000000 +0000
@@ -65,7 +65,7 @@
     "prettier": "^2.0.5",
     "react": "17.0.2",
     "react-native": "0.70.0",
-    "react-native-builder-bob": "^0.20.0",
+    "react-native-builder-bob": "^0.23.1",
     "release-it": "^15.0.0",
     "typescript": "^5.0.2"
   },

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR changes the output of create-react-native-library. You can find the diff of the change below:

diff -r -u --no-ignore-file-name-case old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
--- old-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
+++ new-version/android/src/main/java/com/bob/reactnativetest/ReactNativeTestModule.java
@@ -6,6 +6,7 @@
 import com.facebook.react.bridge.ReactApplicationContext;
 import com.facebook.react.bridge.ReactMethod;
 
+// Hi mom!
 public class ReactNativeTestModule extends ReactNativeTestSpec {
   public static final String NAME = "ReactNativeTest";
 
diff -r -u --no-ignore-file-name-case old-version/babel.config.js new-version/babel.config.js
--- old-version/babel.config.js
+++ new-version/babel.config.js
@@ -1,3 +1,4 @@
 module.exports = {
+  // Hi mom!
   presets: ['module:metro-react-native-babel-preset'],
 };

@atlj atlj force-pushed the @atlj/diff-commentor branch from 37e61e9 to d3d14c7 Compare October 15, 2023 13:59
@atlj atlj changed the title [WIP] chore: comment diffs automatically chore: comment diffs automatically Oct 15, 2023
@atlj atlj requested a review from satya164 October 15, 2023 14:04
@atlj atlj force-pushed the @atlj/diff-commentor branch from 739cfe3 to 73a3a35 Compare October 27, 2023 14:37
@@ -0,0 +1,306 @@
name: Comment template diffs
on:
workflow_dispatch:
Copy link
Member

Choose a reason for hiding this comment

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

you can remove this - it doesn't make sense for commenting diff

Comment on lines +24 to +27
- name: Setup Node.js
uses: actions/setup-node@v3
with:
node-version-file: .nvmrc
Copy link
Member

Choose a reason for hiding this comment

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

this should already be handled by the next job


- name: Remove old build and build again
run: |
rm -rf ./packages/create-react-native-library/lib
Copy link
Member

Choose a reason for hiding this comment

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

running prepare already deletes this folder due to --delete-dir-on-start

Comment on lines +86 to +88
git fetch origin main
git checkout origin/main
git pull || true
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this happen BEFORE installing dependencies and building the library?

@@ -0,0 +1,30 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

let's move this to scripts/ at root. i'm not a fan of files that are not workflows inside the workflows folder

also use snake-case for bash scripts

Comment on lines +90 to +120
create_library() {
library_type=$1
language=$2

echo "Running $library_type/$language"
path_prefix="../test/$library_type/$language"
target_path="$path_prefix/old-version"

npx create-react-native-library "$target_path" \
--slug @bob/react-native-test \
--description test \
--author-name test \
--author-email test@test \
--author-url https://test.test \
--repo-url https://test.test \
--type "$library_type" \
--languages "$language" \
--no-example \
--no-local

# Remove the .git folder of the created library
rm -rf "$target_path/.git"
}

for library_type in "${libraryTypes[@]}"; do
for language in "${languages[@]}"; do
if [[ ! "${exclude[*]}" =~ ${library_type}/${language} ]]; then
create_library "$library_type" "$language"
fi
done
done
Copy link
Member

Choose a reason for hiding this comment

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

this code is the same as before right? in that case maybe you can move this whole thing to the script instead of only having matrix to reduce duplication, or 2 scripts? and pass the folder name to the script which seems to be the only difference

Comment on lines +65 to +71
for library_type in "${libraryTypes[@]}"; do
for language in "${languages[@]}"; do
if [[ ! "${exclude[*]}" =~ ${library_type}/${language} ]]; then
create_library "$library_type" "$language"
fi
done
done
Copy link
Member

Choose a reason for hiding this comment

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

maybe JS script would be better for readability instead of bash

done

- name: Remove everything in the working directory
run: for i in $(ls) ; do rm -rf "$i"; done;
Copy link
Member

Choose a reason for hiding this comment

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

is the loop needed? rm -r * .* should do the job as well, right?

Comment on lines +261 to +273
const replaceTable = {
"module-legacy": "Native module",
"module-mixed": "Turbo module with backward compat",
"module-new": "Turbo module",
"view-legacy": "Native view",
"view-mixed": "Fabric view with backward compat",
"view-new": "Fabric view",
"java-objc": "Java and Objective C",
"java-swift": "Java and Swift",
"kotlin-objc": "Kotlin and Objective C",
"kotlin-swift": "Kotlin and Swift",
".txt": ""
}
Copy link
Member

Choose a reason for hiding this comment

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

having the matrix in JSON would be nice, since this table could also be colocated.

though maybe we should add a command to create-react-native-library, create-react-native-library --list-types and create-react-native-library --list-languages or something that lists all available templates instead hardcoding. then we could use that for matrix in build-templates as well as here. i have forgotten to change the matrix when i change templates in past (worse when something gets added and there's no test failure), and it'll avoid this.

const [type, language] = fileName.split("+");

const title = Object.entries(replaceTable).reduce((acc, [key, value]) => {
return acc.replace(new RegExp(key, "g"), value);
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't return acc.replace(key, value); work here?

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.

2 participants