stevedlawrence commented on code in PR #110:
URL: https://github.com/apache/daffodil-vscode/pull/110#discussion_r867906663


##########
.github/workflows/CI.yml:
##########
@@ -14,41 +14,79 @@
 # limitations under the License.
 
 ---
-name: Code Formatting
+name: Daffodil CI

Review Comment:
   I would actually just suggest `CI`. We did this for Daffodil because in some 
places GitHub displays jobs like `Workflow Name / Job Name`. Sometimes GitHub 
doesn't proide much space for this, so it cuts off the job name, which is the 
part that actually matters. So just changing to "CI" ensures the job is always 
visible.



##########
.github/workflows/CI.yml:
##########
@@ -14,41 +14,79 @@
 # limitations under the License.
 
 ---
-name: Code Formatting
+name: Daffodil CI
 on:
   push:
     branches-ignore: [ 'dependabot/**' ]
   pull_request:
     types: [opened, synchronize, reopened]
 
 jobs:
-  ts-format:
-    name: TypeScript Formatting
+  single-commit:
+    name: Single Commit Pull Request
+    if: github.event_name == 'pull_request'
     runs-on: ubuntu-20.04
     steps:
-      - uses: actions/[email protected]
-      - name: Setup Node
-        uses: actions/[email protected]
+      - name: Check Single Commit
+        uses: actions/[email protected]
         with:
-          node-version: '16'
-      - run: yarn install
-      - run: yarn lint
+          script: |
+            const commits = await github.rest.pulls.listCommits({
+              ...context.repo,
+              pull_number: context.issue.number,
+            });
+            core.info("Number of commits in this pull request: " + 
commits.data.length);
+            if (commits.data.length > 1) {
+              core.setFailed("If approved with two +1's, squash this pull 
request into one commit");
+            }
 
-  scala-format:
-    name: Scala Formatting
+  format-rat-check:
+    name: TypeScript/Scala Formatting and Rat Check
     runs-on: ubuntu-20.04
     defaults:
       run:
         shell: bash
     env:
       SBT: sbt -J-Xms1024m -J-Xmx5120m -J-XX:ReservedCodeCacheSize=512m 
-J-XX:MaxMetaspaceSize=1024m ++2.12.13

Review Comment:
   I believe the newest version of Daffodil needs scala 2.12.15. It will work 
with 2.12.13, but there are some edge cases (I think related to serialized 
parses?) that require the latest version of Scala.



##########
.github/workflows/CI.yml:
##########
@@ -14,41 +14,79 @@
 # limitations under the License.
 
 ---
-name: Code Formatting
+name: Daffodil CI
 on:
   push:
     branches-ignore: [ 'dependabot/**' ]
   pull_request:
     types: [opened, synchronize, reopened]
 
 jobs:
-  ts-format:
-    name: TypeScript Formatting
+  single-commit:
+    name: Single Commit Pull Request
+    if: github.event_name == 'pull_request'
     runs-on: ubuntu-20.04
     steps:
-      - uses: actions/[email protected]
-      - name: Setup Node
-        uses: actions/[email protected]
+      - name: Check Single Commit
+        uses: actions/[email protected]
         with:
-          node-version: '16'
-      - run: yarn install
-      - run: yarn lint
+          script: |
+            const commits = await github.rest.pulls.listCommits({
+              ...context.repo,
+              pull_number: context.issue.number,
+            });
+            core.info("Number of commits in this pull request: " + 
commits.data.length);
+            if (commits.data.length > 1) {
+              core.setFailed("If approved with two +1's, squash this pull 
request into one commit");
+            }
 
-  scala-format:
-    name: Scala Formatting
+  format-rat-check:
+    name: TypeScript/Scala Formatting and Rat Check
     runs-on: ubuntu-20.04
     defaults:
       run:
         shell: bash
     env:
       SBT: sbt -J-Xms1024m -J-Xmx5120m -J-XX:ReservedCodeCacheSize=512m 
-J-XX:MaxMetaspaceSize=1024m ++2.12.13
-    
+
     steps:
       - uses: actions/[email protected]
+      - name: Setup Node
+        uses: actions/[email protected]
+        with:
+          node-version: '16'
       - name: Setup Java
         uses: actions/[email protected]
         with:
           distribution: temurin
           java-version: 11
       - run: $SBT scalafmtCheck
       - run: $SBT scalafmtSbtCheck
+      - run: yarn install
+      - run: yarn lint
+      - name: Run Rat Check
+        run: $SBT ratCheck || (cat target/rat.txt; exit 1)
+      
+  build-test-package:
+    name: Build, Test, and Package
+    strategy:
+      matrix:
+        os: [macos-11, ubuntu-20.04, windows-2019]
+        node: [ '10', '12', '14', '16' ]

Review Comment:
   Seems reasonable to me, but I'm not that familiar with node.
   
   Are there any compatibility issues that might need to be checked for those 
other versions of node?
   
   Does VS Code ship with a specific version of node that our extension needs 
to work with? Or do we just bundle it with the extension? If we bundle, can we 
just test with that specific version? Does the yarn.lock file specify a version 
of node?



##########
.github/workflows/CI.yml:
##########
@@ -14,41 +14,79 @@
 # limitations under the License.
 
 ---
-name: Code Formatting
+name: Daffodil CI
 on:
   push:
     branches-ignore: [ 'dependabot/**' ]
   pull_request:
     types: [opened, synchronize, reopened]
 
 jobs:
-  ts-format:
-    name: TypeScript Formatting
+  single-commit:
+    name: Single Commit Pull Request
+    if: github.event_name == 'pull_request'
     runs-on: ubuntu-20.04
     steps:
-      - uses: actions/[email protected]
-      - name: Setup Node
-        uses: actions/[email protected]
+      - name: Check Single Commit
+        uses: actions/[email protected]
         with:
-          node-version: '16'
-      - run: yarn install
-      - run: yarn lint
+          script: |
+            const commits = await github.rest.pulls.listCommits({
+              ...context.repo,
+              pull_number: context.issue.number,
+            });
+            core.info("Number of commits in this pull request: " + 
commits.data.length);
+            if (commits.data.length > 1) {
+              core.setFailed("If approved with two +1's, squash this pull 
request into one commit");
+            }
 
-  scala-format:
-    name: Scala Formatting
+  format-rat-check:

Review Comment:
   Agreed.



##########
.github/workflows/CI.yml:
##########
@@ -14,41 +14,79 @@
 # limitations under the License.
 
 ---
-name: Code Formatting
+name: Daffodil CI
 on:
   push:
     branches-ignore: [ 'dependabot/**' ]
   pull_request:
     types: [opened, synchronize, reopened]
 
 jobs:
-  ts-format:
-    name: TypeScript Formatting
+  single-commit:
+    name: Single Commit Pull Request
+    if: github.event_name == 'pull_request'
     runs-on: ubuntu-20.04
     steps:
-      - uses: actions/[email protected]
-      - name: Setup Node
-        uses: actions/[email protected]
+      - name: Check Single Commit
+        uses: actions/[email protected]
         with:
-          node-version: '16'
-      - run: yarn install
-      - run: yarn lint
+          script: |
+            const commits = await github.rest.pulls.listCommits({
+              ...context.repo,
+              pull_number: context.issue.number,
+            });
+            core.info("Number of commits in this pull request: " + 
commits.data.length);
+            if (commits.data.length > 1) {
+              core.setFailed("If approved with two +1's, squash this pull 
request into one commit");
+            }
 
-  scala-format:
-    name: Scala Formatting
+  format-rat-check:
+    name: TypeScript/Scala Formatting and Rat Check
     runs-on: ubuntu-20.04
     defaults:
       run:
         shell: bash
     env:
       SBT: sbt -J-Xms1024m -J-Xmx5120m -J-XX:ReservedCodeCacheSize=512m 
-J-XX:MaxMetaspaceSize=1024m ++2.12.13
-    
+
     steps:
       - uses: actions/[email protected]
+      - name: Setup Node
+        uses: actions/[email protected]
+        with:
+          node-version: '16'
       - name: Setup Java
         uses: actions/[email protected]
         with:
           distribution: temurin
           java-version: 11
       - run: $SBT scalafmtCheck
       - run: $SBT scalafmtSbtCheck
+      - run: yarn install
+      - run: yarn lint
+      - name: Run Rat Check
+        run: $SBT ratCheck || (cat target/rat.txt; exit 1)
+      
+  build-test-package:
+    name: Build, Test, and Package
+    strategy:
+      matrix:
+        os: [macos-11, ubuntu-20.04, windows-2019]
+        node: [ '10', '12', '14', '16' ]
+    runs-on: ${{ matrix.os }}
+    defaults:
+      run:
+        shell: bash
+    env:
+      SBT: sbt -J-Xms1024m -J-Xmx5120m -J-XX:ReservedCodeCacheSize=512m 
-J-XX:MaxMetaspaceSize=1024m ++2.12.13
+    steps:
+      - uses: actions/[email protected]
+      - name: Install Node.js
+        uses: actions/[email protected]
+        with:
+          node-version: ${{ matrix.node}}
+      - run: yarn build
+      - run: yarn test
+      - run: $SBT test
+      - run: yarn package
+      - run: $SBT package

Review Comment:
   Looks like `yarn package` is new to the CI and it runs `vsce package`. Is 
that any different than `yarn build`?
   
   Also, `yarn build` will run `sbt universal:packageBin`, which will runs `sbt 
package`, so I think the sbt package line is redundant.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to