Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[+] Added Makefile + Checking script for Go version #222

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

I-S-Tech
Copy link

These commits add :

  • Makefile
  • scripts/verify_go_version.rb

In order to compile in a more "usual" way: running make will check for dependencies and build the project.

Require Ruby in order to check the Go version.

Notes:

  • make only works on linux.
  • Minimum Go version have been set to go1.23.0. You can change this within the Makefile itself.

In order to compile vulnapi, you need Go version go1.23.xx.
Default installation version for Ubuntu is 1.18.1.
Using Makefile, you can now check for go version when compiling.
You can also run "make" in order to build, and "make clean" in order to
clean.
This is a more common way to build.
Not required to build / run vulnapi
Copy link
Member

@emmanuelgautier emmanuelgautier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contributions and very sorry for the delay to give you feedback about this PR.

I agree that using a Makefile can be good thing, particularly for more complex projects to build or for someone not familiar with the language build toolchain.

This project is currently quite straightforward to build (thanks to Go for that). Introducing this Makefile adds a specific build process and introduces Ruby and shell dependencies.

Maybe I'm missing the primary purpose of this PR. What issue are you aiming to solve? Did you encounter any issue while building the project?

@@ -0,0 +1,22 @@
FROM ubuntu:22.04
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a Dockerfile here: https://github.com/cerberauth/vulnapi/tree/main/.docker

Why would be the purpose for this new Dockerfile?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though having a "global" Dockerfile, aimed at "contained build and run" could be usefull, but you're absolutely right, there is no need for another Dockerfile.

@I-S-Tech
Copy link
Author

I-S-Tech commented Dec 5, 2024

I did not encounter particular issue regarding the build (except issues with the go version, thanks to the Ubuntu Repositories).

The idea behind the PR was to add the standard Makefile build tool, in order to have a straightforward way to build, and prevent other people from having the same "issue" with go version.

That said, the issue probably lies in the fact that I am not particularly familiar with Go.

@emmanuelgautier
Copy link
Member

Hello @I-S-Tech,

I understand now the main purpose of the PR, but I think it is more of a Go installation issue. I am not comfortable maintaining a dedicated Go installation inside the project for different OS, distributions, etc. IMHO This should be more the main purpose of the Go project itself.

I am considering adding a contributing documentation with more details about how to starting contributing, including pointing to the right resources for Go installation. I have just opened #236 to add some ideas about this. Feel free to add your ideas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants