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

Update docs/contributing #4406

Open
geyslan opened this issue Dec 2, 2024 · 9 comments · May be fixed by #4424
Open

Update docs/contributing #4406

geyslan opened this issue Dec 2, 2024 · 9 comments · May be fixed by #4424
Assignees
Milestone

Comments

@geyslan
Copy link
Member

geyslan commented Dec 2, 2024

Tracee dev env got evolving but without updating the respective https://github.com/aquasecurity/tracee/tree/main/docs/contributing sections.

For instance:

  • make check-pr
  • make format-pr
  • VM_TYPE=test vagrant up
  • make -f builder/Makefile.man {man,man-build,man-run}
  • make -f builder/Makefile.protoc protoc-{build,run}
  • make -f builder/Makefile.performance dashboard-{start,stop}
  • github comment: /fast-forward
  • etc
@geyslan geyslan added this to the v0.23.0 milestone Dec 2, 2024
@geyslan geyslan self-assigned this Dec 2, 2024
@ShohamBit
Copy link
Collaborator

@geyslan could you clear a bit more what changes should be made?

@geyslan
Copy link
Member Author

geyslan commented Dec 9, 2024

@geyslan could you clear a bit more what changes should be made?

A full improvement or revamp of the contributing pages. For that one would read all docs, test already explained commands/steps. In the meantime to check if the listed above are missing or are not so well explained. If you feel that a restructure of pages is required, it's ok, let us know how you plan to redesign it.

@ShohamBit
Copy link
Collaborator

@geyslan could you clear a bit more what changes should be made?

A full improvement or revamp of the contributing pages. For that one would read all docs, test already explained commands/steps. In the meantime to check if the listed above are missing or are not so well explained. If you feel that a restructure of pages is required, it's ok, let us know how you plan to redesign it.

Ok i will go over the relevant docs and rebuild it as needed

is make format-pr the same like make fix-fmt?

@geyslan
Copy link
Member Author

geyslan commented Dec 9, 2024

format-pr prints a standard log that we use in the PR header message.

check-pr calls it at its final stage as well, after checking files.

In other words, the latter ignores checking.

@ShohamBit
Copy link
Collaborator

fix-fmt is not called when you call check-pr, so i gona add 2 section in the source code guidelines

@yanivagman yanivagman assigned ShohamBit and unassigned geyslan Dec 10, 2024
@ShohamBit
Copy link
Collaborator

@geyslan I need you to clarify some things:

  1. is the Makefile.checkers is up-to-date?
  2. I didn't find docs for the following Makefile:
  • man
  • k8s
  • performance
  • protoc
  • release
    where should I document them

Also, I want to inform you that I remove the Documentation section and combine it with the guidelines

@geyslan
Copy link
Member Author

geyslan commented Dec 11, 2024

@ShohamBit

@geyslan I need you to clarify some things:

  1. It is updated.
  2. You can insert them in the already existent sections or put them together in a new one. It will depend on the subject, by instance, man (man files), k8s (operator) and protoc (grpc) concern to the contributor work, on the other hand release is handled only by maintainers.

Also, I want to inform you that I remove the Documentation section and combine it with the guidelines

Ok.

@yanivagman @NDStrahilevitz @rscampos FYI

@ShohamBit
Copy link
Collaborator

@geyslan when I call

make -f ./builder/Makefile.checkers 

it shows this help :

To check formatting you should run:

    $ make -f builder/Makefile.checkers fmt-check

To fix formatting you should run:

    $ make -f builder/Makefile.checkers fmt-fix

To check code you should run:

    $ make -f builder/Makefile.checkers code-check

Note: you should run fmt-fix before doing a git commmit.
Note: clang-format-12 is needed for eBPF code checks/fixes.

do we want to keep it like this or change it to the format of check-pr

@geyslan
Copy link
Member Author

geyslan commented Dec 11, 2024

I believe that check-pr would do, as it already call the other. But we lack of a high level fix-something in the main Makefile. It would be nice to have both at same level.

@ShohamBit ShohamBit linked a pull request Dec 11, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants