upsidr / merge-gatekeeper

Get better merge control
MIT License
92 stars 15 forks source link

Makefile and developer docs #53

Closed Jrc356 closed 1 year ago

Jrc356 commented 1 year ago

Overview

This PR creates some useful docs on how to develop merge-gatekeeper. It also adds a makefile containing some useful commands for building, running, and testing

Details

Jrc356 commented 1 year ago

not really sure why importer for markfdown fails here :( @rytswd any ideas?

rytswd commented 1 year ago

Sorry for getting back late, I wasn't too well for the past several days...

Regarding the Importer error, I tried it in my local, it looked as if the new markdown file needed a blank line at the end. I tried to push a commit to fix, but it seemed to have gone to our repo branch - I was hoping it would be picked up in the PR, but it looks like it's not. Could you check your fork and see if you can pull in my commit to your branch?

The change itself looks good, so once the CI is happy, I'd be happy to get this merged 🥰

Jrc356 commented 1 year ago

@rytswd doesn't seem like a new line fixed it. Looks like the job is failing to install imported as it's trying to use brew without brew being installed. Lemme see if I can patch that

rytswd commented 1 year ago

Ah OK, sorry that's a known issue, there is a fix for that. Let me dig up the fig we use internally.

rytswd commented 1 year ago

@Jrc356 Can you replace the part for brew install with the following YAML steps - that should do the trick

      - name: Setup Go
        uses: actions/setup-go@v3
        with:
          go-version: '>=1.18.0'
          check-latest: true
      - name: Install Importer
        run: go install github.com/upsidr/importer/cmd/importer@v0.1.4
Jrc356 commented 1 year ago

@rytswd I can do that... I just pushed a new change that uses macos as the build agent for that job instead. Would you prefer your suggestion instead?

rytswd commented 1 year ago

Ah hmm, as this is an Open Source project and GitHub Action is generous about the Open Source usage, that's certainly one solution. But I think it would be better to stick to Ubuntu for the basic configuration. It would be great if you could try my fix to see if that one works as well (also it should be slightly faster than brew command).

Jrc356 commented 1 year ago

@rytswd no worries! Updated and successfully run. Thanks for the assist!

rytswd commented 1 year ago

Going ahead with the merge, thanks for your contribution @Jrc356 🥰

rytswd commented 1 year ago

Btw, I will be making a new release, probably tomorrow. That should contain a bunch of fixes created from you in the recent days 🥰