Add Makefile (failing linters disabled)

c0d1ngm0nk3y commented 9 months ago

fixes #260 first chunk of #293

Summary

This pr adds a Makefile and also fixes some linting errors so that make lint will run without error.

With the exception that some checks had been deactivated, because the errors were beyond the scope of this.

Use Cases

It's standard in a lot of Go projects. It also makes it easier to run test, lint, and CI.

Checklist

[X] I have viewed, signed, and submitted the Contributor License Agreement.
[X] I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
[ ] I have added an integration test, if necessary.
[X] I have reviewed the styleguide for guidance on my code quality.
[X] I'm happy with the commit history on this PR (I have rebased/squashed as needed).

anthonydahanne commented 8 months ago

@pivotal-david-osullivan , @dmikusa , are we settled on this way of running linting? Personally, I'm fine with it, even if I'm still not a big fan of Makefile in general

dmikusa commented 8 months ago

@anthonydahanne I think that's a fair question, and worth taking a step back and reviewing.

Here's what I can share.

I've been told that using a Makefile is somewhat common in Go projects.
We use a Makefile in libcnb and it's used in other CNB projects too. There's a template there in terms of commands that we could follow and get some parity with CNB.
I don't have any particular fondness for Makefile, the reason I created the issue to do this is because 1.) and 2.).
If folks are not familiar with Makefile, it's probably not a big deal cause the complexity of what's done there is low.
The other buildpacks team just uses pure shell scripts for builds. No Makefiles as far as I know. If we were to not go with Makefile, then using their build scripts would make the most sense.
I run the Go commands to test/build and it works out well too. There are not that many and they are not that complicated, so I suppose we could just do nothing too. This isn't something that we get a ton of complaints about either.

@anthonydahanne and @c0d1ngm0nk3y - Having said all that, I'm open to suggestions. Thoughts?

dmikusa commented 8 months ago

@c0d1ngm0nk3y One thing I noticed, that would probably be good to do in this PR, is to remove Richgo. The author no longer recommends using it, so I suspect that it could break or mess up our pipelines. https://github.com/kyoh86/richgo?tab=readme-ov-file#notice-what-i-think-about-richgo

c0d1ngm0nk3y commented 7 months ago

One thing I noticed, that would probably be good to do in this PR, is to remove Richgo.

Sure. I can do this. I just wonder because it is used all over the place (e. libcnb, lifecycle).

c0d1ngm0nk3y commented 7 months ago

Personally, I'm fine with it, even if I'm still not a big fan of Makefile in general

What is your preferred way? I think it is easy, quite common and I find it very refreshing if related projects (e.g. libcnb, lifecycle) how something in common.

linux-foundation-easycla[bot] commented 7 months ago

The committers listed above are authorized under a signed CLA.

:white_check_mark: login: c0d1ngm0nk3y / name: Ralf Pannemans (9ef286527563373836f52255d5920ed737abaf25)

c0d1ngm0nk3y commented 7 months ago

btw: When rebase, I had to comment out just another linter because of issues. So there is a real benefit in having a linter running in the pr validation.

c0d1ngm0nk3y commented 7 months ago

I've been told that using a Makefile is somewhat common in Go projects.

That is my impression as well.

We use a Makefile in libcnb and it's used in other CNB projects too. There's a template there in terms of commands that we could follow and get some parity with CNB.

That is for me the killer argument. It feels very strange to switch between CNB project and out of the sudden things are done differently.

I don't have any particular fondness for Makefile, the reason I created the issue to do this is because 1.) and 2.).

What would be the alternative? tbh, I do not see a single benefit in doing something special for this project.

If folks are not familiar with Makefile, it's probably not a big deal cause the complexity of what's done there is low.

Exactly, the complexity is low. So getting familiar with this Makefile is not too hard, imho.

The other buildpacks team just uses pure shell scripts for builds. No Makefiles as far as I know. If we were to not go with Makefile, then using their build scripts would make the most sense.

But libcnb and lifecycle do use a Makefile.

I run the Go commands to test/build and it works out well too. There are not that many and they are not that complicated, so I suppose we could just do nothing too. This isn't something that we get a ton of complaints about either.

But running a linter in pr validation is long overdue, isn't it?

dmikusa commented 7 months ago

I don't have any particular fondness for Makefile, the reason I created the issue to do this is because 1.) and 2.).

What would be the alternative? tbh, I do not see a single benefit in doing something special for this project.

I don't really have strong opinions on this. I mostly said this in case others have strong opinions, to encourage them to speak up. You make good arguments for Makefile, so happy to go along with that.

I run the Go commands to test/build and it works out well too. There are not that many and they are not that complicated, so I suppose we could just do nothing too. This isn't something that we get a ton of complaints about either.

But running a linter in pr validation is long overdue, isn't it?

Agreed. So long as it's not making busy work for the project. We've got plenty of things to do, don't need more work :) It's probably a good target for the v2's, to have clean linted code.