You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/22 09:55:53 UTC

[GitHub] [pulsar-test-infra] mangoGoForward opened a new pull request, #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

mangoGoForward opened a new pull request, #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75

   Signed-off-by: mango [xu.weiKyrie@foxmail.com](mailto:xu.weiKyrie@foxmail.com)
   
   This PR add action of check-pr-title to ensure your PR title matches the Pulsar Pull Request Naming Convention Guide. And more test scenes, please see the https://github.com/mangoGoForward/C/pull/8


-- 
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: dev-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] nodece closed pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
nodece closed pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide
URL: https://github.com/apache/pulsar-test-infra/pull/75


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] nodece commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
nodece commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1272730872

   Thank you all for your contribution! Closed by https://github.com/apache/pulsar/pull/17961.


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] tisonkun commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1272491411

   As @nodece suggested, use the action solution at https://github.com/apache/pulsar/pull/17961.
   
   @mangoGoForward I think it's just the original patch you want to make. No need to maintain a new handmade action as proposed here.


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] tisonkun commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1261862308

   I believe this feature is separated from the original `docbot` functionality.
   
   If I implement this proposal, I will simply use standard Conventional Commit + GitHub Apps like https://github.com/Ezard/semantic-prs which are easy to implement and easy to tell.
   
   One patch adds a `.github/semantic.yml` to the main repo. One ticket asks for the INFRA team to turn on the App.
   
   It works well for projects like [bytebase](https://github.com/bytebase/bytebase/blob/main/.github/semantic.yml) and most of my own projects. I learned from the INFRA team that there're already other Apache projects that use it but I don't find a specific one.
   
   `type`s and `scope`s can be configured, the pattern need not be customized. No new actions. No burden on the workflow runs queue. No code.
   
   I will vote -1 on this implementation, non-binding. And I already talked multiple times about the background:
   
   * https://lists.apache.org/thread/ltjt37wr21sj2t33kgsotxmmm8935f5b
   * https://github.com/apache/pulsar/pull/16836#discussion_r932995700
   * https://github.com/apache/pulsar/pull/16836#discussion_r934089250
   
   If you have your reason, as I comment on https://github.com/apache/pulsar/pull/17557#discussion_r966796899, do what you like and see the result.


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] tisonkun commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1272705941

   As https://github.com/apache/pulsar/pull/17961 merged, @mangoGoForward you can close ths PR and https://github.com/apache/pulsar/pull/17557 as implemented the same functionality.
   
   Thanks for your pioneer work!


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] mangoGoForward commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
mangoGoForward commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1261984830

   > @mangoGoForward Could you also provide corresponding tests in your own forked pulsar repo, and put links under this PR?
   
   @maxsxu Please look at mangoGoForward/pulsar#2
   
   - (Valid title: not match headerPattern)PR title: `fix: add more env to test pr title checker`, the `Documentation Bot / Check PR title ` job failed, and `github-actions` add a comment like "@mangoGoForward Please follow the [Pulsar Pull Request Naming Convention Guide](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#bookmark=id.y8943h392zno) to write your PR title."
   - (Valid title: error scope)PR title: `[fix][cc] Add more env to test pr`, the `Documentation Bot / Check PR title ` job failed, and `github-actions` add a comment like "@mangoGoForward Please follow the [Pulsar Pull Request Naming Convention Guide](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#bookmark=id.y8943h392zno) to write your PR title."
   - (Valid title: missing scope and not match headerPattern)PR title: `[fix] Add more env to test pr title`, the `Documentation Bot / Check PR title ` job failed, and `github-actions` add a comment like "@mangoGoForward Please follow the [Pulsar Pull Request Naming Convention Guide](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#bookmark=id.y8943h392zno) to write your PR title."
   - (Valid title: missing type)PR title: `[][ci] Add more env to test pr title`, the `Documentation Bot / Check PR title ` job failed, and `github-actions` add a comment like "@mangoGoForward Please follow the [Pulsar Pull Request Naming Convention Guide](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#bookmark=id.y8943h392zno) to write your PR title."
   - (Valid title)PR title: `[feat][ci] Add more env to test pr title`, the `Documentation Bot / Check PR title ` job successful


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] mangoGoForward commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
mangoGoForward commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1260653955

   > 
   
   Got~


-- 
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: dev-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] lhotari commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1261733349

   To be honest, I don't think that it's a great idea to use Golang for GitHub Actions. 
   
   Javascript/Typescript would be a much better choice since there's a lot less overhead of running the actions and another benefit is that there's official GitHub provided Javascript libraries for writing actions.
   
   A Golang action will have to be compiled before the code can run. With Javascript/Typescript actions there's no such requirement since it can execute immediately.
   
   Who can explain the reason for using Golang?


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] mangoGoForward commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
mangoGoForward commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1255744688

   @maxsxu Please to a review when you have free time, 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: dev-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] maxsxu commented on a diff in pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
maxsxu commented on code in PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#discussion_r983613677


##########
docbot/README.md:
##########
@@ -50,10 +50,87 @@ jobs:
 ## Configurations
 
 | Name                    | Description                            | Default                   |
-| ----------------------- |----------------------------------------| ------------------------- |
-| `GITHUB_TOKEN`          | The GitHub Token                       | &nbsp;                   |
+|-------------------------|----------------------------------------|---------------------------|
+| `GITHUB_TOKEN`          | The GitHub Token                       | &nbsp;                    |
 | `LABEL_PATTERN`         | RegExp to extract labels               | `'- \[(.*?)\] ?`(.+?)`' ` |
-| `LABEL_WATCH_LIST`      | Label names to watch, separated by `,` | &nbsp; |
+| `LABEL_WATCH_LIST`      | Label names to watch, separated by `,` | &nbsp;                    |
 | `ENABLE_LABEL_MISSING`  | Add a label missing if none selected   | `true`                    |
-| `LABEL_MISSING`         | The label mssing name                  | `label-missing` |
-| `ENABLE_LABEL_MULTIPLE` | Allow multiple labels selected         | `false`                   |
\ No newline at end of file
+| `LABEL_MISSING`         | The label mssing name                  | `label-missing`           |
+| `ENABLE_LABEL_MULTIPLE` | Allow multiple labels selected         | `false`                   |
+
+
+# action-check-pr-title
+This is a [GitHub Action](https://github.com/features/actions) that ensures your PR title matches the Conventional Commits spec.
+
+The typical use case is to use this in combination with a tool like semantic-release to automate releases.
+
+## Installation
+
+1. [Add the action](https://docs.github.com/en/actions/quickstart) with the following configuration
+```yml
+name: "Check PR title"
+
+on:
+  pull_request_target:
+    types:
+      - opened
+      - edited
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  label:
+    if: ${{ github.repository == 'apache/pulsar' }}
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout action
+        uses: actions/checkout@v3
+        with:
+          repository: apache/pulsar-test-infra
+          ref: master
+
+      - name: Set up Go
+        uses: actions/setup-go@v3
+        with:
+          go-version: 1.18
+
+      - name: Labeling
+        uses: ./docbot title check
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

Review Comment:
   ```suggestion
     title-check:
       runs-on: ubuntu-latest
       steps:
         - name: Validating PR Title
           uses: apache/pulsar-test-infra/docbot/cmd/title-check@master
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
   ```



-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] tisonkun commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1271664704

   Let me take a concrete technical review of the implementation design, and show you how it makes no sense to me.
   
   **Point 1.** The simplest solution is as commented https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1261862308.
   
   > One patch adds a `.github/semantic.yml` to the main repo. One ticket asks for the INFRA team to turn on the App.
   
   **Question.** So, why do we come to the current situation? From the "necessity" perspective, the implementation deals with two arguments.
   
   **Point 2.** You want to run our own pattern instead of the Conventional Commit standard bundled by https://github.com/Ezard/semantic-prs.
   
   Here you hit the point I already argued several times that a new pattern _generates_ requirements to implement and maintain a new wheel. The next question is, is it worth the effort?
   
   I would say most developers don't care what standard is in use but understandable and consistent. CI failed for an explicit reason. I know how to fix it and if I follow the simple convention, it won't bother me anymore. That's good.
   
   **Point 3.** You want to comment on the PR thread. That's what is different from an identical fork of https://github.com/amannn/action-semantic-pull-request.
   
   Again, it _generates_ requirements and we ask whether it is worth the effort.
   
   @lhotari wrote a workflow that significantly changes all developers' workflow https://github.com/apache/pulsar/pull/17693. It only leaves a summary in the check result and no one complains about it yet. Again, developers generally don't care what the standard is - CI failed, failure reasons, taking actions, CI passed, and cheers.
   
   https://github.com/Ezard/semantic-prs can redirect you to `targetUrl` if you click the check state failure "details" button. If we rewrite a self-documented `.github/semantic.yml` file, it fits all the effective requirements.
   
   **Point 4.** I don't have a preference between Golang and JavaScript. But this feature cannot be mangled into the existing docbot - they are different things. Also, scheduling new GitHub Actions runs consume CI resource which we'd better avoid.
   
   I'm going to follow the solution I proposed above to see if it can be a smooth path as I described. I don't think it's a big deal to argue or who takes the credit - we solve the issue in a way that most developers fit.


-- 
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@pulsar.apache.org

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

[GitHub] [pulsar-test-infra] mangoGoForward commented on pull request #75: feat: add action of check-pr-title to ensure PR title matches Naming Convention Guide

Posted by GitBox <gi...@apache.org>.
mangoGoForward commented on PR #75:
URL: https://github.com/apache/pulsar-test-infra/pull/75#issuecomment-1272205748

   > To be honest, I don't think that it's a great idea to use Golang for GitHub Actions.
   > 
   > Javascript/Typescript would be a much better choice since there's a lot less overhead of running the actions and another benefit is that there's official GitHub provided Javascript libraries for writing actions.
   > 
   > A Golang action will have to be compiled before the code can run. With Javascript/Typescript actions there's no such requirement since it can execute immediately.
   > 
   > Who can explain the reason for using Golang for these actions?
   
   @lhotari Thanks for your review. 
   As far as I know, [Github Actions supported language include C, C++, C#, Go, Java, JavaScript, PHP, Python, Ruby, Scala, and TypeScript](https://docs.github.com/en/get-started/learning-about-github/github-language-support). And I want to describe the reason why I choose Go to implement this feature:
   
   - I'm a Go language developer, and the technical solution is open for an open-source project.
   - There are many Github Actions using Go to implement the feature they want.
   - Before I start to coding, we discussed that we can implement it in `docbot` to reduce code repositories @maxsxu @Anonymitaet , we can use subcommand like `./docbot title check` in workflow, we introduced `Cobra` because `Cobra` is a library for creating powerful modern CLI applications and used in many Go projects such as [Kubernetes](https://kubernetes.io/), [Hugo](https://gohugo.io/), and [GitHub CLI](https://github.com/cli/cli) to name a few.


-- 
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@pulsar.apache.org

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