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

fix: Mismatch of PWD in tutorial #283

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

kenoss
Copy link
Contributor

@kenoss kenoss commented Sep 7, 2021

What is fixed

At this line, readers are in <repository-root>/tutorial, while the youki binary is in <repository-root>.
I fixed the mismatch.

@yihuaf
Copy link
Collaborator

yihuaf commented Sep 7, 2021

Thank you for the catch. But can I ask you to change the tutorial to be done in the root, instead of tutorial? In another word, can we remove https://github.com/containers/youki/pull/283/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R110
and update the path to be from the project root?

Edit: actually let's try this and see if it make better sense to people this way.

@codecov-commenter
Copy link

Codecov Report

Merging #283 (0803ee9) into main (1770f34) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #283      +/-   ##
==========================================
+ Coverage   69.61%   69.62%   +0.01%     
==========================================
  Files          46       46              
  Lines        5660     5660              
==========================================
+ Hits         3940     3941       +1     
+ Misses       1720     1719       -1     

@yihuaf yihuaf merged commit c247c90 into youki-dev:main Sep 7, 2021
@kenoss kenoss deleted the fix-pwd-mismatch-in-tutorial branch September 7, 2021 20:34
@kenoss
Copy link
Contributor Author

kenoss commented Sep 7, 2021

Thanks for your review and suggestion. An obstruction for the suggestion is youki spec can only generate config.json in the current directory and it needs additional mv. I feel the current version is simple enough, so I agree with your conclusion.

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.

3 participants