openembedded / meta-openembedded

MIT License
402 stars 700 forks source link

android-tools-adbd.service: Change /var to /etc in ConditionPathExists #851

Closed quic-raghuvar closed 2 months ago

quic-raghuvar commented 2 months ago

If android-tools-adbd.service service needs to be up upon boot, then the path assigned to ConditionPathExists must be present at boot time. This means that the path set to ConditionPathExists must be created at build time itself. In this case it is var/usb-debugging-enabled. However, /etc is a better place to keep files and directories that are created at build time rather than /var. /var is expected to house files that are created at run time.

Hence, change ConditionPathExists=/var/usb-debugging-enabled to ConditionPathExists=/etc/usb-debugging-enabled

This helps enablement of android-tools-adbd.service from build by creating /etc/usb-debugging-enabled at build time.

kraj commented 2 months ago

applied to master with eb31674b1c050ebccf0c85f4accc15cc7ea4b58a

quic-raghuvar commented 1 month ago

@kraj Can this patch be pulled to krikstone branch as well?

kraj commented 1 month ago

Send a pr against Kirkstone or better on mailing list

quic-raghuvar commented 1 month ago

Done. Created a PR against Kirkstone. https://github.com/openembedded/meta-openembedded/pull/857

lumag commented 1 month ago

@kraj you have merged the patches (this one and #857), however they don't have DCO (Signed-off-by), they have a lame name in the Author field ("quic-raghuvar") instead of the full name, the commit messages is a text dump which is not wrapped on the useful text width.

Please consider putting more attention to the GitHub PRs, they can come from undereducated developers and can have mistakes which are easy to miss thanks to the GH interface.

kraj commented 1 month ago

@lumag thanks for noting it, I think it slipped in due to oversight. I think because this pull pushed the change after the effect. @quic-raghuvar please follow

https://docs.yoctoproject.org/dev/contributor-guide/submit-changes.html

to prepare patches with good commit msg and patch headers in future.

lumag commented 1 month ago

@kraj maybe we should add GitHub actions to do some checks (at least format, DCO, etc). Maybe we can use TuxSuite / TuxBake to perform build-testing. This should help reviewers.