libs icon indicating copy to clipboard operation
libs copied to clipboard

Coding style for `libs` repo

Open Andreagit97 opened this issue 2 years ago • 23 comments

Motivation

Hi folks! I think we have to introduce a definitive coding style for this repository. The number of lines continuously grows and sometimes becomes difficult also to understand the code without a specific standard! In this repo, we already have a configuration file for clang-format and VScode offers an amazing extension that allows us to format our file in a very simple way. The rationale here is to introduce a CI/CD job that checks the coding style of every new Pull request and denies the merge until the code respects the conventions. The initial discussion is started here but I think we need a dedicated thread for it. It would be great to put together some ideas here!

Feature

Introduce a CI/CD job that checks the coding style of every new Pull request and denies the merge until the code respects the conventions.

Andreagit97 avatar Jun 08 '22 09:06 Andreagit97

cc @leogr @Molter73

Andreagit97 avatar Jun 08 '22 09:06 Andreagit97

Big +1 from me, having coherent code is always nice. I also have no strong opinions on what the style should be, so the existing clang-format file looks good to me. I do feel strongly that style must be properly enforced through a CI job as stated.

Further more, in order to not force people to use vscode for formatting, we might want to consider adding some sort of make target or script that runs clang-format locally so validation before committing is easily done. As an example, this is how we handle code formatting in stackrox/collector: https://github.com/stackrox/collector/blob/b3ff69b6d5a86eba00c25a8569a79c2f77546612/collector/Makefile#L64-L74. Bonus points if we provide a pre-commit hook for checking style so contributors don't have to do the mental exercise of remembering to format the code before committing.

Molter73 avatar Jun 08 '22 10:06 Molter73

Big +1 from me, having coherent code is always nice. I also have no strong opinions on what the style should be, so the existing clang-format file looks good to me. I do feel strongly that style must be properly enforced through a CI job as stated.

Further more, in order to not force people to use vscode for formatting, we might want to consider adding some sort of make target or script that runs clang-format locally so validation before committing is easily done. As an example, this is how we handle code formatting in stackrox/collector: https://github.com/stackrox/collector/blob/b3ff69b6d5a86eba00c25a8569a79c2f77546612/collector/Makefile#L64-L74.

We absolutely need a script or a make target!

Bonus points if we provide a pre-commit hook for checking style so contributors don't have to do the mental exercise of remembering to format the code before committing.

This would be amazing in my opinion! :rocket:

Andreagit97 avatar Jun 08 '22 10:06 Andreagit97

This might be useful: https://github.com/muttleyxd/clang-tools-static-binaries

deepskyblue86 avatar Jun 08 '22 15:06 deepskyblue86

I would also propose to change the current formatting to avoid tabsize 8, the code gets too wide (personal taste)

deepskyblue86 avatar Jun 16 '22 10:06 deepskyblue86

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana avatar Sep 14 '22 15:09 poiana

/remove-lifecycle stale

jasondellaluce avatar Sep 15 '22 08:09 jasondellaluce

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana avatar Dec 14 '22 09:12 poiana

/remove-lifecycle stale

leogr avatar Dec 14 '22 10:12 leogr

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana avatar Mar 14 '23 15:03 poiana

/remove-lifecycle stale

Andreagit97 avatar Mar 14 '23 17:03 Andreagit97

/milestone 0.12.0

FedeDP avatar Apr 27 '23 09:04 FedeDP

Proposing to schedule this and commit to it (also the reviewers) and then agree to a v1 we actually merge. WDYT @Andreagit97 we definitely need this?

incertum avatar Aug 23 '23 18:08 incertum

I suggest scheduling this for immediately after we secure the Falco 0.36 release. Approximately between mid-Sept and early Oct. wdyt?

leogr avatar Aug 24 '23 10:08 leogr

I suggest scheduling this for immediately after we secure the Falco 0.36 release. Approximately between mid-Sept and early Oct. wdyt?

Sounds perfect to me!

incertum avatar Aug 24 '23 17:08 incertum

@Andreagit97 and @leogr kindly checking in on this as it's Nov 🙃 ty!

incertum avatar Nov 13 '23 04:11 incertum

At the moment I have no time to work on this and moreover, we are near the 0.14.0 release, unfortunately, it is not a great moment to go on with this :(

Andreagit97 avatar Nov 13 '23 07:11 Andreagit97

👍 Let's discuss in our next strategy meeting how we can support each other to get this over the finish line.

incertum avatar Nov 13 '23 19:11 incertum

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana avatar Feb 11 '24 21:02 poiana

/remove-lifecycle stale

Andreagit97 avatar Feb 13 '24 09:02 Andreagit97

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale

poiana avatar May 13 '24 09:05 poiana

/remove-lifecycle stale

Andreagit97 avatar May 13 '24 14:05 Andreagit97

May we revamp this once 0.38 is out? :thinking:

leogr avatar May 14 '24 08:05 leogr