Add Makefile and fix some linting issues

paketo-buildpacks / libpak

An opinionated extension to the libcnb Cloud Native Buildpack Library

Apache License 2.0

15 stars 18 forks source link

Add Makefile and fix some linting issues #293

Closed c0d1ngm0nk3y closed 9 months ago

c0d1ngm0nk3y commented 1 year ago

fixes #260

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).

c0d1ngm0nk3y commented 1 year ago

@paketo-buildpacks/java-maintainers The pr got a bit larger because of make make lint work. So there is some potential for conflicts. And new stuff could break the linting again. WDYT?

c0d1ngm0nk3y commented 11 months ago

One question could be what about the failing linters commented out in golangci.yml. But I would argue, it is better to have this already in and then tackle the remaining linter errors with issues - maybe even one linter at a time.

Any thoughts?

dmikusa commented 11 months ago

One question could be what about the failing linters commented out in golangci.yml. But I would argue, it is better to have this already in and then tackle the remaining linter errors with issues - maybe even one linter at a time.

Any thoughts?

+1 for having it included. I know there are going to be issues we need to fix related to lints because we haven't been running them. Hopefully having it in there will help identify and fix them. If we could make it just WARN initially and not fail the build I think that would be good. Then we can correct them later & make it start failing the build so we don't introduce new lint issues.

c0d1ngm0nk3y commented 11 months ago

@paketo-buildpacks/java-maintainers How to continue with this?

dmikusa commented 11 months ago

That's a good question. I think this is waiting on one of the @paketo-buildpacks/java-maintainers to review. I've been meaning to take a look but it keeps sliding down in my queue as other more urgent things pop up. I will try to review this though as it's important for v2. Thanks for your patience on this.

c0d1ngm0nk3y commented 10 months ago

Thanks for your patience on this.

Maybe I can open an alternative pr to this that purely adds the Makefile. Commenting out all linters that currently fail. That should keep the pr much smaller. Once it is merged, on linter could be enabled after the other together with the fixes. Sound like I should have done it in the first place tbh. WDYT?

see #301

c0d1ngm0nk3y commented 9 months ago

close in favour for https://github.com/paketo-buildpacks/libpak/pull/301 (easier to review)