Melkeydev / go-blueprint

Go-blueprint allows users to spin up a quick Go project using a popular framework
MIT License
2.07k stars 141 forks source link

Logger #191

Closed mcheviron closed 2 days ago

mcheviron commented 2 months ago

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

Problem/Feature

Issue #182

Description of Changes:

This PR introduces a logger template and updates the main template to include the logger. Here are the detailed changes:

Checklist

mcheviron commented 2 months ago

You are using 'log.info' only in 'main.go'. Could you include logging across all templates where it is appropriate? I thought that was the initial idea?

OK. That's why I wanted to clarify what I was going to do in prev comment because I thought all of them use the same main.go template. I see now that fiber uses a different one, that's it or are there other templates to take into consideration?

Documentation updates are also needed. You can add the new tree structure in 'index.md' and provide a brief explanation in 'creating-project.md'.

I'll add it. I just saw it

Please check why tests are not passing

I didn't read the CI/CD pipeline so I'm still unaware of what it does. I'll take a look and if I'm stuck I'll ping you.

Cheers!

PS: Please ping me if i'm missing anything else. Thanks!

mcheviron commented 2 months ago

@Ujstor Hi again. After an illegal amount of time I think I know why the CI/CD pipeline is angry at me:

jobs:
  install_dependencies:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: Setup Go
        uses: actions/setup-go@v4
        with:
          go-version: "1.20"
      - name: Install Dependencies
        run: go mod download
  framework_matrix:
    needs: install_dependencies
    strategy:
      matrix:
        framework:
          [chi, gin, fiber, gorilla/mux, httprouter, standard-library, echo]
        driver:
          [mysql, postgres, sqlite, mongo, redis, none]
        goVersion: ["1.20"]

slog was experimental in 1.20 and the actual release is in 1.21

That's why I get this fuckery:

 Running [/home/runner/golangci-lint-1.55.2-linux-amd64/golangci-lint run --out-format=github-actions --path-prefix=chi --timeout=5m] in [/home/runner/work/go-blueprint/go-blueprint/chi] ...
  level=warning msg="[runner] Can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for \"log/slog\""
  level=error msg="Running error: 1 error occurred:\n\t* can't run linter goanalysis_metalinter: buildir: failed to load package : could not load export data: no export data for \"log/slog\"\n\n"

  Error: golangci-lint exit with code 3
  Ran golangci-lint in 412ms

I think. I tried act on my local machine but it kind of worked so I'm not sure if I'm missing something. Anyways, what do you believe we should do?

Also added the template to fiber as well. I just need to create a subdirectory in the internal directory and that's all, I believe. Cheers

Ujstor commented 2 months ago

@mcheviron

I believe I may have misunderstood some things around implementation. I envision an implementation where, after a user creates a 'go-blueprint' project, a custom logger is automatically integrated into all generated template files (such as 'server.go,' 'database.go,' 'routes.go,' etc., depending on the frameworks, databases, or advanced options selected).

This approach would provide users with comprehensive examples of how the logger is utilized throughout the entire project. Consequently, users would gain a better understanding of its usage and be able to implement it more effectively in their own projects

Check cons in program.go and logic how new dir/subdir is created.

In generate-linter.yml, we can change the Go version, and the linter would not encounter any problems.

Sorry for delay, I am little busy lately.

mcheviron commented 2 months ago

Sorry for delay, I am little busy lately.

I totally understand. I am as well. I think I have a better understanding of what you want and I think it's a good idea. I'll try to get into it as soon as possible and if I feel like I'm being too opinionated, i'll comment out the code but leave it there in case the user wishes to uncomment it.

Do you want me to update the linter myself? I believe updating the framework_matrix task would solve the issue but since it's the CI pipeline, I didn't want to touch it without bringing it up. Cheers

Ujstor commented 2 months ago

Yeah, you can update the Go version in the linter, and that should do the trick.

Your logger is fine, use it in other packages and replace old logs with the new implementation. The current one is not ideal:

log.Fatalf(fmt.Sprintf("db down: %v", err))

Don't rush with code, there is another big PR that introduces websockets, and there are a lot of changes in templates. We can find the best way to implement it after ws PR is merged.

Hit me up on Discord in DM; my username is the same, and we can discuss and find the best solution.

mcheviron commented 2 months ago

Hit me up on Discord in DM; my username is the same, and we can discuss and find the best solution.

Hi sorry for the delay. I tried pinging you but you can't be added via strangers.

Yeah, you can update the Go version in the linter, and that should do the trick.

Your logger is fine, use it in other packages and replace old logs with the new implementation. The current one is not ideal:

I will generate some projects this weekend and reread our convo to make sure I put the logger in the right subdir and then use it throughout the projects. If you add me, i'll ping you there before I commit.

P.S: I'll update the linter version as well

mcheviron commented 2 months ago

ci/cd is updated to use 1.22 while linting. it's better to sync the versions, 1.21.1 is used somewhere (that's just a suggestion, do what's best for the proj)

logger is put in its own pkg under internal

i added a Fatal method to the logger, it called log.Fatal under the hood. slog offers no API for fataling

if you want it to be used in the db initialisation for all the templates, we'd either do a dependancy injection on the New() for each db and use the logger therein. but since log.Fatal is used inside, I don't know if it's worth it. but it's doable

Ujstor commented 2 days ago

I am closing this repo, assuming there is no interest to continue implementation.