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

add link for libsystemd-journal-dev install #384

Open
wants to merge 1 commit into
base: master
from

Conversation

@zhangxiaoyu-zidif
Copy link
Member

zhangxiaoyu-zidif commented Nov 8, 2019

No description provided.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Nov 8, 2019

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zhangxiaoyu-zidif
To complete the pull request process, please assign dchen1107
You can assign the PR to them by writing /assign @dchen1107 in a comment when ready.

The full list of commands accepted by this bot can be found 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

@k8s-ci-robot k8s-ci-robot added the size/XS label Nov 8, 2019
@k8s-ci-robot k8s-ci-robot requested review from andyxning and Random-Liu Nov 8, 2019
@xueweiz

This comment has been minimized.

Copy link
Contributor

xueweiz commented Nov 8, 2019

I'd actually prefer to not add this link :) because the link points to a Ubuntu Trusty (14.04) repository. But the developer could be using any Ubuntu version, and this link could mislead them.

What do you think?

Also, I think providing the package name as is OK for now. If you want it to be more clear, I think a good way would be to shutdown more gracefully.

One way to do that would be to add another rule in the makefile, (e.g. "check dependency"). The rule will check whether these requirements are met, and fail with a clear & actionable error message ;)

@zhangxiaoyu-zidif

This comment has been minimized.

Copy link
Member Author

zhangxiaoyu-zidif commented Nov 8, 2019

check dependency is fine. I prefer it also.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.