VilnaCRM-Org / user-service

Creative Commons Zero v1.0 Universal
5 stars 1 forks source link

fix(#38): cannot make commit after project initialization #39

Open paaneko opened 1 month ago

paaneko commented 1 month ago

Description

Fix issues that prevent developers from making commits if they don't have PHP installed on their local machines.

Related Issue

resolves #38

Types of changes

Checklist:

paaneko commented 1 month ago

Because there is no php container running inside the php container, so make commands from the Makefile cannot be run inside the container. In the commit I hardcoded the commands from Makefile and it doesn't look very cool. Maybe someone knows how to make it more flexible?

Kravalg commented 1 month ago

You need to run the application using the make start command and then commit and push your changes

paaneko commented 1 month ago

Done. But can u please explain why i should run make start before pushing? Very interesting)

Kravalg commented 1 month ago

Because we run all CI checks in the one environment - docker container with the php Make start will run the php container, then you can develop a new feature or fix the bug, test it, run locally all CI checks that you need, and then commit and push changes, which will be tested by captainhook in the same container where you did the changes

Kravalg commented 1 month ago

@paaneko can we close this PR ?

paaneko commented 1 month ago

Yes, on mac this issue fixed.

paaneko commented 1 month ago

Oh, I didn’t notice that the commits with my changes were not merged into main, so the problem remains. Problem is that the captainhook uses PHP installed on the local machine, and not Docker one. And my changes fix it. Even if i run make start command, captainhook will throw error in console that php not found on local machine.

paaneko commented 1 month ago

We little misunderstood each other :)

Kravalg commented 1 month ago

Please check the Makefile, we are using the docker compose by default for make commands that we are running in the captainhook @Derane can you check this bug additionally ?

paaneko commented 1 month ago

It can't be done because CaptainHook will call make commands inside the Docker container, and it requires another Docker container inside container for make commands. Unfortunately, this container does not exist.

Possible solution would be to create a Docker-in-Docker image, which the first container CaptainHook will use to run built-in commands and captainhook scripts that require PHP dependency. For example:

"actions": [
    {
        "action": "\\CaptainHook\\App\\Hook\\Message\\Action\\Regex",
        "options": {
            "regex": "/^(feat|fix|refactor|build|chore|ci|doc|style|perf|test)\\(\\#\\d+\\):.*$/i",
            "error": "The commit message should be in the format: feat|fix|refactor|build|chore|ci|doc|style|perf|test(#1):"
        }
    }
]

However, to call make commands, it needs a nested php container inside the first php container.

The solution I provide in this PR is to run check commands without the docker compose exec php prefix, so the command will run on the first Docker container without requiring nested one.

Kravalg commented 1 month ago

Thanks a lot, @Derane will check this bug

Derane commented 1 month ago

Thanks a lot, @Derane will check this bug

Yeah, @paaneko is right. PHP installed locally really must have for commit.