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

[Trivial] Fix makefile targets to use PHONY #1743

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

yihuaf
Copy link
Collaborator

@yihuaf yihuaf commented Mar 29, 2023

Not all makefile target in Youki is using .PHONY target. We don't use any file targets, so fixing it properly for all makefile.

On a different note, there has been a lot of popularity in using justfile as command runner to invoke different command, especially for Rust projects. Since we use Makefile only as a command runner, I want to bring justfile up to see if there is any interest. Note, Makefile is definitely good enough for us. The upside is that justfile is a little more ergonomic being used as a command runner. The downside is that we have to install justfile as part of the dependency, whereas Makefile is more readily available. Food for thought :)

Reference: https://github.com/casey/just

@yihuaf yihuaf marked this pull request as ready for review March 29, 2023 16:27
@yihuaf yihuaf requested review from YJDoc2 and utam0k March 29, 2023 16:28
@yihuaf yihuaf self-assigned this Mar 29, 2023
@yihuaf yihuaf added the enhancement New feature or request label Mar 29, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #1743 (8a8b8e3) into main (04de7bf) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1743   +/-   ##
=======================================
  Coverage   68.69%   68.69%           
=======================================
  Files         121      121           
  Lines       13314    13314           
=======================================
  Hits         9146     9146           
  Misses       4168     4168           

Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

I didn't know case/just. I'll check it. Thanks

@utam0k utam0k merged commit e498e0f into youki-dev:main Mar 30, 2023
@yihuaf yihuaf deleted the yihuaf/makefile branch March 30, 2023 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants