Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Expose a GitHub Action #335

Closed
thejcannon opened this issue Feb 23, 2023 · 20 comments · Fixed by #337
Closed

[FEATURE] Expose a GitHub Action #335

thejcannon opened this issue Feb 23, 2023 · 20 comments · Fixed by #337
Labels
enhancement New feature or request

Comments

@thejcannon
Copy link

Is your feature request related to a problem? Please describe.
As someone who doesn't do much web coding, and so isn't as familiar with the relevant steps to "set up" this tool, I'd love to run this tool in a GHA, witha "turnkey" experience.

Describe the solution you'd like
An easy-to-use configurable GitHub Action.

Describe alternatives you've considered
Gotta roll my own

Additional context
<3

@thejcannon thejcannon added the enhancement New feature or request label Feb 23, 2023
@gagoar
Copy link
Owner

gagoar commented Feb 23, 2023

Sounds good. Lemme sketch this out a little bit. and get back to you. If you have any suggestions for it. lemme know!

@gagoar
Copy link
Owner

gagoar commented Feb 27, 2023

Hey @thejcannon! I was wondering, I could create an action that uses the generator and what not, but it will only generate the files, you will have to add something to the workflow to commit that to the PR say (if is the context you are working on).

to summarize:

The action could run the codeowners-generator and produce the CODEOWNERS file, but something else in the workflow would have to add that to the PR or the main branch.

I can provide the example in the documentation for sure. Just wondering if that would be "turnkey" ready for you.

thanks in advance!

@thejcannon
Copy link
Author

Sounds perfect, and extensible.

I can make a workflow that checks the file matches expected, and one that pushes a generated result that's opt in

@gagoar
Copy link
Owner

gagoar commented Feb 27, 2023

Okay then. I will add the action.yml here, and see how it goes. Ping you when I have something concrete. :)

@gagoar
Copy link
Owner

gagoar commented Feb 28, 2023

Alright, take a look at #337. I've test it and it does the job. my tiny workflow for the test was this:

name: Lint

on:
  push:
    branches:
      - main
  pull_request:

jobs:
  codeowners: 
    runs-on: ubuntu-latest
    needs: getNodeVersion
    steps:
      - uses: actions/checkout@v2 # to checkout the code of the repo you want to change the CODEOWNERS from. 
      - name: running codeowners
        uses: gagoar/codeowners-generator@gg/action
        with:
          use-maintainers: true

I can add the generate token and commit to the PR for instance. Lemme know if you need more guidance !

@thejcannon
Copy link
Author

Yup that looks right on par. Thanks for the speedy resolution 🙌

@gagoar
Copy link
Owner

gagoar commented Feb 28, 2023

🎉 I will clean things up and add Documentation. Lemme know if you would like to help with the use case you were looking for in the form of Readme! :)

@TansyArron
Copy link
Contributor

One helpful feature would be the option to pass a flag to have it report failure when the generator produces a diff - this would make it easier to fail a PR if people have updated codeowners without regenerating. As it is I'm running this action with a following job:

    - name: check if codeowners wrote a diff
        # The length of the output of `git diff` is 0 when there are no changes.
        # Check the length of the git diff and exit 1 if the length is not 0.
        run: if [[ $(git diff | wc -c) -ne 0 ]]; then $(exit 1); fi

Thanks for the quick work on putting this together!

@gagoar
Copy link
Owner

gagoar commented Mar 3, 2023

I see what you mean. The only issue with putting this in the action is the number of assumptions I will have to do. Particularly assuming someone did a actions/checkout.

Also, not producing a change in CODEOWNERS is not necessarily a bad thing in many chases. Say you ran it locally, and when is running in the cloud it doesn't find a difference. In any case.

I think the best we can do to keep the action agnostic is the following:

  • If I find a CODEOWNERS file, I will sha5 that file. Then run the tool and compare the shas. I would be able to provide an output from the tool that will say CODEOWNERS_CHANGED.

  • I will add an option making the action error out when CODEOWNNERS_CHANGED has errored out.

I need help naming the option. Any suggestions? FAIL_ON_DIFF?

@thejcannon
Copy link
Author

FWIW You usually see this kinda thing on the tool itself. Usually the code itself is isolated to the step that does the writing (e.g. "fail if check else write").

Example: https://prettier.io/docs/en/cli.html#--check

@thejcannon
Copy link
Author

Otherwise probably not worth dirtying the action over, callers can find their own solution (like git diff --porcelain)

@gagoar
Copy link
Owner

gagoar commented Mar 3, 2023

I believe it could be a good idea to have the action as a guard or to use the action to commit the changes if it finds any changes as I've done in some projects. This is useful when a project is not JS-centric.

So I can sort of see the use case @thejcannon.

@thejcannon
Copy link
Author

thejcannon commented Mar 3, 2023

Oh I'm advocating for the new option in the action. But I'm also adding that it should be an option on the tool itself.

To lay all the cards out here:

  • @TansyArron and I maintain a Python monorepo
  • Currenty we try and generate CODEOWNERS (poorly, and from a singular yml file)
  • Your tool looks like a very nice way to make CODEOWNERS easy to maintain, while keeping it intuitive and flexible
  • The current thought would be to use the action on every PR and on merge_group. If the action is running on PR, push the changes (if any) to the PR branch. If it's running on merge_group, error on changes.

So we'd be using the action for the checking, because we don't already use JS. But for folks who ARE invoking the tool directly (everyone today, since the action doens't exist yet) --check on the tool could help them too.

@gagoar
Copy link
Owner

gagoar commented Mar 3, 2023

Thanks for the context!

So, you would not want the codeowners-generator to generate but just to validate if what it contains is correct?

Adding a check on the cli will be no problem. then making that option available within the action is like the preview one.

Confirm that I've understood what you are asking before I start coding :)

@thejcannon
Copy link
Author

Yes. I mean technically I want both. But being able to control the behavior from an option means I have both. So yeah <3

@gagoar
Copy link
Owner

gagoar commented Mar 3, 2023

hehe, yea, this gives you both. Without a doubt.

alright. Got my marching orders it seems. I will ping you guys back with the PR a bit later this weekend. Thanks for staying on top of the 🧵

@thejcannon
Copy link
Author

No, thank you. You're doing the hard work 😉

@gagoar
Copy link
Owner

gagoar commented Mar 7, 2023

Hey, @thejcannon and @TansyArron. Just to keep you updated, the feature is up for review in #342 As soon as I get someone to review it (hopefully we get the wonderful @gustavkj to 👀 ) I will fast forward with the action, which it will involve adding the option :)

gagoar added a commit that referenced this issue Mar 8, 2023
After a bit of discussion (mainly #335) we narrowed it down into a
solution that will simplify the implementation of the action.

The functionality `--check` does something very simple: It fails if the
existing CODOWNERS file doesn't match the generated one. It will be
useful to "lint" if the committed code is in fact matching what it
should be. This functionality as said previously will simplify the
action given that many developers seem to want just the check and not to
generate the update within the workflow (Like I would have thought
initially).

I will provide documentation, but for now the code is complete and
tested (unit test)
@TansyArron
Copy link
Contributor

Thank you!

gagoar added a commit that referenced this issue Mar 19, 2023
This PR will add the the capability for the project to be an action in
GitHub.

I will fast follow this PR with a release so I can publish it on the
GitHub market.

This PR should close #335
@gagoar
Copy link
Owner

gagoar commented Mar 19, 2023

Hey @TansyArron @thejcannon I just published the action here 🙏🏻 take a look if can! and try it now from the tag itself (uses: gagoar/[email protected]) or just pointing to master

I've also added some documentation considering 2 use cases: linting (using the check) and committing changes directly to the PR itself.

Thanks again!

@gagoar gagoar mentioned this issue Mar 22, 2023
gagoar added a commit that referenced this issue Mar 23, 2023
Bumping version 2.2.0
That comes with:
- Preserve block position
[`#347`](#347)
- ChatGPT rewrote it
[`#349`](#349)
- Using swc instead of ts-jest
[`#348`](#348)
- Update Node.js to v16.19.1
[`#341`](#341)
- Update dependency husky to v8.0.3
[`#339`](#339)
- adding composite action to use codeowners-generator in a workflow
[`#337`](#337)
- 2.1.5
[`#346`](#346)
- Preserve block position (#347)
[`#343`](#343)
- adding composite action to use codeowners-generator in a workflow
(#337)
[`#335`](#335)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants