lgeiger / black-action

A GitHub action that runs black code formatter for Python
MIT License
190 stars 35 forks source link

Provide a complete example? #6

Open mlissner opened 4 years ago

mlissner commented 4 years ago

I'm currently struggling to get this set up over in https://github.com/mlissner/black-tests. It seems like it should be so simple, but getting the github action figured out is really hard, apparently. I'm sort of just batting around in the dark. Making progress, but with much frustration.

Would it be possible to provide an example repository using this action? Or to have it run on this repository?

Thanks for the great tool. I hope I can get it to work!

mlissner commented 4 years ago

If you're amenable to the idea of a sample project, I can share that I finally got mine working. The idea is to do a very minimal black project before doing it in a larger project, so it might be ideal as an example: https://github.com/mlissner/black-tests/blob/master/.github/workflows/pythonapp.yml

Thank you again, and sorry for the issue-spam!

KonstantinSchubert commented 4 years ago

@mlissner Hi, thank you for your example, it is really helpful to me.

Can you help me answer a question:

How does the un-linted code get INTO the docker container and how does the linted code get OUT of the docker container?

I feel like there is some magic going on that I am not getting?

mlissner commented 4 years ago

In my example:

name: black

on: [push]

jobs:
  black:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - name: Black Code Formatter
        uses: lgeiger/black-action@master
        with:
          args: ". --check"

The checkout command gets your code. I don't know how black, which is inside a docker container, is able to access it, but it is. The above works. So I guess black just looks for code in the current directory and is off to the races.

how does the linted code get OUT of the docker container

It doesn't. The way this one works it just throws an error if the code is changed by black. The expectation is that you run black before you push your code, and this Github action just enforces that by exploding when you fail to do that properly.

KonstantinSchubert commented 4 years ago

@mlissner Thanks for taking the time to answer me.

I must be missing something fundamental because in my understanding, a process that runs in a docker container isn't able to access data that exists outside the docker container.

So how on earth does this docker process manage to lint the code that's outside the docker container?

mlissner commented 4 years ago

It's magic. No, really. I got curious and dug around and it doesn't seem to be documented anywhere, but this is just how it works. The entrypoint command just has access to your codebase through some auto-mount or other that Github does for you. Good fun!

(I just sent a note to their contact form to fix this in their documentation.)

shahzebsiddiqui commented 4 years ago

I am having similar issue, i tried to setup a similar GitHub action to make changes on push or pull_request. See https://github.com/HPC-buildtest/buildtest-framework/blob/devel/.github/workflows/black.yml

When i got this configured and looking at my actions. I get a black screen saying the following:

image

See: https://github.com/HPC-buildtest/buildtest-framework/commit/3c7381155ae039839e17510d71113fdd20fda415/checks?check_suite_id=332081301

At first it would be nice to just report the error and definitely the new PR (https://github.com/lgeiger/black-action/pull/2) to automate push via black would be nice. I dont know the implication though. I understood that GitHub action will make the format changes during the push. It is not clear if the GitHub action is intercepted during the push so that Github can modify the code and retain same commit such as git commit --amend or just push on top of HEAD. If there is a push, dont there need to be a git author. I anticipate that whats being done in line https://github.com/lgeiger/black-action/pull/2/files#diff-b958f585a04af5ee2087610ea7180f34R30-R31

mlissner commented 4 years ago

I think discussion about making changes on push go on the other ticket. This ticket is about having an example configuration. I think you might benefit by looking at mine, by the way (linked above). Looks like part of your issue is you didn't pull the code before trying to run black.

shahzebsiddiqui commented 4 years ago

So i updated my github action as follows:

name: Black Formatter
on: [push, pull_request]
jobs:
  black-format:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v1
      - name: Black Code Formatter
        uses: lgeiger/black-action@master
        with:
          args: ". --check"

Question is do i need the --check as i know this doesn't apply the changes. I do like the feature of having this before letting black make changes to all files. Is there way i can get black to format all code automatically after check is successful. I don't want risk of automatically applying change causing something to break after formatting.

Probably worth checking error code after each command or maybe setting set -e in script somehow. Not sure how to approach this.

image

mlissner commented 4 years ago

Is there way i can get black to format all code automatically after check is successful.

I don't understand. If the code needs formatting, it'll fail the check? Why would you want to format the code after a check is successful?

shahzebsiddiqui commented 4 years ago

should I remove the section

with:
  args: ". --check"

to disable check. Is there an example github action that i can reference. I'd like to get rid of this error I am getting from docker.

Flamefire commented 4 years ago

Can you please first make up your mind what you want exactly and then explain that? (see https://github.com/lgeiger/black-action/issues/6#issuecomment-561264944)

args are the arguments passed to black. So read the block docu on what to pass there depending on what you want.