Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

ci: add auto-me-bot to validate conventional pr titles #120

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dolby360
Copy link
Contributor

@TomerFi Please take a look :)

@dolby360
Copy link
Contributor Author

@dimabru Please note that you'll have to install the app. You can find docs here

@dolby360 dolby360 force-pushed the dolev/enforce_pr_title_conventions branch 2 times, most recently from 9712987 to 22a86af Compare October 15, 2022 18:30
Copy link

@TomerFi TomerFi left a comment

Choose a reason for hiding this comment

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

LGTM

@dimabru
Copy link
Contributor

dimabru commented Oct 18, 2022

@TomerFi @dolby360
I have to ask. Verifying pr title conventions should be quite simple in my opinion.
Do we really need a github app for that? can't we solve it with a simple github action?

@dolby360
Copy link
Contributor Author

@dimabru unfortunately, that's how the project works. If you decide to keep away from any app installation we will have to find another tool.

@TomerFi
Copy link

TomerFi commented Oct 18, 2022

I have to ask. Verifying pr title conventions should be quite simple in my opinion.
Do we really need a github app for that? can't we solve it with a simple github action?

Of course we can.
Off the top of my head, this can be done with a shell script or with github-script action using javascript.
Though the later might get complicated because it is inline javascript.
I also assume there are already github actions ready for this, and if not, this should be quite simple to create one.

If any of the above is the direction you choose, feel free to ping me if any help is required.

As for me, well... I've been avoiding apps and sticking to actions for a long time, a while back I've reached the conclusion that, for me, workflows require more maintenance then apps, and apps (if you write them yourself, which is what I did) can be much more dynamic then workflows.

BTW, that's why I called it "auto-me-bot", because it was designed to replace me by doing stuff I used to do manually via workflows. 😉

EDIT:
Thinking about it a bit more, using javascript with github-script might get very messy as we use a dependency for linting, commitlint.

@dolby360
Copy link
Contributor Author

@TomerFi Can you please elaborate on how we can use github actions?

@TomerFi
Copy link

TomerFi commented Oct 18, 2022

@dolby360 You know what...
I was going to suggest shell scripting, but I just thought of something way better.
If I'm right and this works, you'll love it!

hang-on let me verify.

@TomerFi
Copy link

TomerFi commented Oct 18, 2022

well...
yes - and no.

yes because

I was able to run in with a simple two line ci step, you can take a look here.

  1. Create a configuration file named commitlint.config.js in the project's root (based on commitlint docs):
module.exports = { extends: ['@commitlint/config-conventional'] };
  1. Then, this simple workflow will do the job:
---
name: Pull request build

on:
  pull_request:
  
jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - run: |
          npm i -g commitlint @commitlint/config-conventional
          echo "${{ github.event.pull_request.title }}" | commitlint

A failed run example can be found here.
A successful run example can be found here.

no because

Based on what I know, without doing too much research on the topic,
I'm not sure there's a way to trigger a workflow based on PR title change, meaning the failing workflow will not re-run the title is changed, a maintainer with the proper permissions will have to manually go into the run and re-run it.

Maybe I'm wrong on this one though, it perhaps worth proper research.

@dolby360
Copy link
Contributor Author

@TomerFi I don't think that any tool will rerun automatically if PR title is changed.
PR workflow shall be run only if a new commit arrived

@TomerFi
Copy link

TomerFi commented Oct 18, 2022

That's a pro for using the app.

@dolby360
Copy link
Contributor Author

@TomerFi Sure but without using any app I think this is a fine compromise

@dolby360 dolby360 force-pushed the dolev/enforce_pr_title_conventions branch from 22a86af to a60002c Compare October 19, 2022 19:24
@dolby360
Copy link
Contributor Author

@dimabru done.
see tests here

@TomerFi
Copy link

TomerFi commented Oct 19, 2022

I think it will better placed if a separate workflow.
That way, after fixing a PR title, you can restart just that specific workflow.

@dolby360
Copy link
Contributor Author

dolby360 commented Oct 20, 2022

@TomerFi I also think this is a good idea. @dimabru what do you think?
@dimabru BTW I'm not sure if you are following. The current solution does not require any app installation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants