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 the test with kind to github action #2027

Merged
merged 1 commit into from
Jun 11, 2023
Merged

Conversation

utam0k
Copy link
Member

@utam0k utam0k commented Jun 9, 2023

No description provided.

@utam0k utam0k force-pushed the ci-kind branch 4 times, most recently from 6906c98 to 76ab585 Compare June 9, 2023 13:26
Copy link
Collaborator

@yihuaf yihuaf left a comment

Choose a reason for hiding this comment

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

With regarding the path issue in justfile, we usually keep track of two path. One is the project root through git rev-parse and another is the justfile is located, through justfile_directory.

TOP := `git rev-parse --show-toplevel`
CWD := {{ justfile_directory() }}

Usually these two location is all we need to run all relative path into an absolute path. Usually, we want the justfile executed at the location of the justfile. Where we invoke the just command should not matter. In this way, all command has a fixed and predictable path.

Otherwise LGTM.

justfile Outdated Show resolved Hide resolved
justfile Outdated Show resolved Hide resolved
justfile Outdated Show resolved Hide resolved
@utam0k
Copy link
Member Author

utam0k commented Jun 10, 2023

With regarding the path issue in justfile, we usually keep track of two path. One is the project root through git rev-parse and another is the justfile is located, through justfile_directory.

TOP := `git rev-parse --show-toplevel`
CWD := {{ justfile_directory() }}

Usually these two location is all we need to run all relative path into an absolute path. Usually, we want the justfile executed at the location of the justfile. Where we invoke the just command should not matter. In this way, all command has a fixed and predictable path.

Otherwise LGTM.

I do not want to leave the case for using git. This is because of the dependence on git. We want to reduce the dependency. Also, I feel it is difficult to judge how to use with justfile_directory. I honestly don't know much of the difference. can you tell me when ROOT is needed?

$ just -f ../justfile youki-dev
/home/utam0k/ghq/src/github.com/utam0k/youki/scripts/build.sh -o /home/utam0k/ghq/src/github.com/utam0k/youki -c youki
* FEATURES:
* features:
    Finished dev [unoptimized + debuginfo] target(s) in 0.07s

Signed-off-by: utam0k <k0ma@utam0k.jp>
@yihuaf
Copy link
Collaborator

yihuaf commented Jun 11, 2023

I do not want to leave the case for using git. This is because of the dependence on git. We want to reduce the dependency.

I see. Is it because you are running the justfile inside the container so git is required?

Also, I feel it is difficult to judge how to use with justfile_directory. I honestly don't know much of the difference. can you tell me when ROOT is needed?

ROOT is needed when you want to reference a specific directory relative to the youki project root, especially if inside subdirectory justfiles. git is the only way to deterministically obtain the project root from anywhere inside. We can certainly workaround not using git, but it will probably requires us to keep track of relative path of where justfiles are inside the subdir and using ../ to get to project root.

I think if the current PR works as it stands, we should merge this and refactor justfile in follow ups. No need to block this PR further on jsutfile refactors since we will do those anyway.

Copy link
Collaborator

@yihuaf yihuaf left a comment

Choose a reason for hiding this comment

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

For the nits with regarding to justfile, we will deal with those in future refactoring. Let's get the test into CI first.

@utam0k
Copy link
Member Author

utam0k commented Jun 11, 2023

I see. Is it because you are running the justfile inside the container so git is required?

I had imaged the case in GH actions.

@utam0k utam0k merged commit 2bd6ebc into youki-dev:main Jun 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants