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

feat(userspace/falco): print version at startup #1303

Merged
merged 1 commit into from
Jul 16, 2020
Merged

Conversation

leogr
Copy link
Member

@leogr leogr commented Jul 7, 2020

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area engine

What this PR does / why we need it:

See #1302

Which issue(s) this PR fixes:

Fixes #1302

Special notes for your reviewer:

/milestone 0.24.0
/cc @fntlnz
/cc @leodido
/cc @kris-nova

Does this PR introduce a user-facing change?:

new(userspace/falco): print the Falco and driver versions at the very beginning of the output.

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
@poiana poiana added this to the 0.24.0 milestone Jul 7, 2020
@leogr leogr changed the title wip: feat(userspace/falco): print version at startup feat(userspace/falco): print version at startup Jul 8, 2020
Copy link
Member

@leodido leodido left a comment

Choose a reason for hiding this comment

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

SGTM!

Anyway, I have a question.

Why do not stick with the usual two-line version (line 626-627) logs we use for the --version flag in order to be more consistent?

There's a particular reason to print Falco version and driver version this way when Falco runs?

@leogr
Copy link
Member Author

leogr commented Jul 15, 2020

SGTM!

Anyway, I have a question.

Why do not stick with the usual two-line version (line 626-627) logs we use for the --version flag in order to be more consistent?

There's a particular reason to print Falco version and driver version this way when Falco runs?

Lines 626-627 uses printf, here I use the initialized logger. Moreover, I have found just one-line output briefer.
No other reasons.
Let me know if you prefer a two-line, and I will change it shortly.

@leodido
Copy link
Member

leodido commented Jul 15, 2020

I don't have particular preferences honestly.

I was just asking if there was a ratio behind putting both versions on the same line. That's all!

@poiana
Copy link

poiana commented Jul 15, 2020

LGTM label has been added.

Git tree hash: b0db7668fb16d54f0db4053d64d0e429d0647fc0

@leodido
Copy link
Member

leodido commented Jul 16, 2020

Moving this to next release because it hasn't reached 2 approvals

/milestone 0.25.0

@poiana poiana modified the milestones: 0.24.0, 0.25.0 Jul 16, 2020
@leodido
Copy link
Member

leodido commented Jul 16, 2020

/hold

@poiana
Copy link

poiana commented Jul 16, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leodido

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@fntlnz
Copy link
Contributor

fntlnz commented Jul 16, 2020

Approved, I think we can get this in now!

@leodido
Copy link
Member

leodido commented Jul 16, 2020

/hold cancel

@poiana poiana merged commit 4346e98 into master Jul 16, 2020
@poiana poiana deleted the feat/log-version branch July 16, 2020 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the Falco version at the very beginning of the log
4 participants