You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/12/06 19:12:57 UTC

[GitHub] [daffodil-vscode] Shanedell opened a new pull request #61: 1 Commit and Code Formatting CI

Shanedell opened a new pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61


   Updates:
   
   - Create CI for checking PRs are 1 commit.
    
   Closes #46


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] jw3 commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
jw3 commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-1013508407


   Looks good to me.  Looks like you need to resolve a conflict in build.sbt though.


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-987138444


   The unit tests CI also run a `lint` before the tests do we want to remove this before testing since linting will be its own CI stage?


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-998905679


   Yeah that makes sense I don't think it is necessary to add the VSIX file to a github release if it is being deployed to marketplace.
   
   I think even if we want to add a VSIX file to Github, for any reason, all that needs done is for someone to run a `yarn build` and upload the VSIX so it wouldn't be much of a hassle.


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] stevedlawrence commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-998879586


   Note that we can distribute through github releases, but it must only be for convenience (like how the marketplace is allowed but only for convenience). "Official" releases must be made available via downloads.apache.org from our website. In general, I thin apache perfer that githug releases not be used in favor of website or something like a marketplace, because github will automatically create "release" in some cases, and it can be confusing what is actually an official release that was voted on.
   
   The release process should be something like:
   1. Create a zip of the source and a vsix file that is expected to be the final release. The .vsix is considered a convenience binary. 
   2. This is called an "-rc1", but rc1 doesn't actually appear anywhere in the source or the convenience binary. Usually it's just the name of a directory in an ASF svn repo (e.g. https://dist.apache.org/repos/dist/dev/daffodil/3.2.1-rc2/) staging files for a vote.
   3. We vote on releasing the source and the convenience .vsix binary
   4. When the vote passes, we rename the directory to remove the -rc1 label, and then we can upload the convenience binaries where it is most convenient. For example, in Daffodil this is maven central, our webiste, and an RPM repo. For vsix ideally it would be the marketplace, and maybe github releases if it makes sense.


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] stevedlawrence commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-987182342


   Please do, thanks!


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] mbeckerle commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-998851553


   re: VISX file. That should not be created and distributed from github. 
   
   Build from source is the right approach for unofficial or between-releases VISX files. If the tool chain for this is too daunting, we can use the approach of giving a docker container and script to build it, but I think it was as simple as just 'yarn build'. 
   


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] jw3 commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
jw3 commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-1013497099


   What's left here?  Would be nice to have the 1-commit check in master.


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] stevedlawrence commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-987195272


   Regarding the release, I'm not sure we even want to keep release.yml. ASF is pretty strict about pre releases not being published outside of dist.apache.org while the release is voted on. And the process to make a pre-release an official release is basically a `mv` from the staging area to the release area. We may need to rethink how releases are build and published.
   


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell edited a comment on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell edited a comment on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-987138444


   The unit tests CI also run a `lint` before the tests do we want to remove this before testing since linting will be its own CI stage? This seems to causes issues when running on unit tests on Windows I assume it has to deal with the line endings being unix not dos maybe? Because the lint run fine for both Mac and Ubuntu but fails for Windows


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell merged pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell merged pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61


   


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-1013499497


   @jw3 I don't think anything just second approval then squash and merge


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on a change in pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on a change in pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#discussion_r772607855



##########
File path: .github/workflows/formatting.yml
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+---
+name: Code Formatting
+on:
+  push:
+    branches-ignore: [ 'dependabot/**' ]
+  pull_request:
+    types: [opened, synchronize, reopened]
+
+jobs:
+  ts-format:
+    name: TypeScript Formatting
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2.3.5
+      - name: Setup Node
+        uses: actions/setup-node@v1.4.6
+        with:
+          node-version: '16'
+      - run: yarn install
+      - run: yarn lint

Review comment:
       Well `yarn lint` is just being removed from the yarn `pre-test` script as linting on a Windows machine causes issues because of line ending differences.
   
   Other than that `prettier` is very customizable with ignoring. If we want to ignore a entire directory, file, type of file, etc you can that to the `.prettierignore` file. Then you can ignore specific lines of file (JavaScript/TypeScript) like this:
   ```
   // prettier-ignore
   ```
   
   for other file types, eg yaml, they detail that here https://prettier.io/docs/en/ignore.html
   




-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell edited a comment on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell edited a comment on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-998844218


   @stevedlawrence I think the main point of the `release.yml` is to create a release/provide a VSIX in Github.
   
   So I think we would want to keep this if we would still like to create VSIX files that can be downloaded by visiting Github. However, if we do not want to do this if the rules around creating a release is strict then I can update the PR to remove the file. Then the only way to run the extension is by installing it through the VSCode marketplace or building the source code locally.
   
   I believe the latter is the way it is done for `openwhisk-vscode-extension` but was not sure if this project would have the same workflow or not. What are your thoughts?


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] mbeckerle commented on a change in pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#discussion_r772601126



##########
File path: .github/workflows/formatting.yml
##########
@@ -0,0 +1,54 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+---
+name: Code Formatting
+on:
+  push:
+    branches-ignore: [ 'dependabot/**' ]
+  pull_request:
+    types: [opened, synchronize, reopened]
+
+jobs:
+  ts-format:
+    name: TypeScript Formatting
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/checkout@v2.3.5
+      - name: Setup Node
+        uses: actions/setup-node@v1.4.6
+        with:
+          node-version: '16'
+      - run: yarn install
+      - run: yarn lint

Review comment:
       How customizable is this lint tool? E.g., can we make it ignore some things that we decide are ok stylistically?




-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-998844218


   @stevedlawrence I think the main point of the `release.yml` is to create a release in Github.
   
   So I think we would want to keep this if we would still like to create VSIX files that can be downloaded by visiting Github. However, if we do not want to do this if the rules around creating a release is strict then I can update the PR to remove the file. Then the only way to run the extension is by installing it through the VSCode marketplace or building the source code locally.
   
   I believe the latter is the way it is done for `openwhisk-vscode-extension` but was not sure if this project would have the same workflow or not. What are your thoughts?


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-998862753


   @mbeckerle Okay in that case I will delete the release.yml from the repo.


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-987154754


   @stevedlawrence I believe you had mentioned before about maybe not doing a different a seperate file for each CI action, instead making just one similar to how the Daffodil does. Would you want me to make an issue for once this one is merged that we work on converging into all of the seperate CI files into one `CI.yml` file?


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [daffodil-vscode] Shanedell commented on pull request #61: 1 Commit and Code Formatting CI

Posted by GitBox <gi...@apache.org>.
Shanedell commented on pull request #61:
URL: https://github.com/apache/daffodil-vscode/pull/61#issuecomment-987186842


   @stevedlawrence Sounds good. Just to let you know I believe we can converge all the files in `CI.yml` except for `release.yml`. This mostly being because it will require a different branch/tag matching than all the other CI items need.
   
   But also what do you think about also deleting the `.eslint` files and dependencies since we are going with prettier?
   


-- 
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: commits-unsubscribe@daffodil.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org