Skip to content

Add raw flag #252

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Add raw flag #252

wants to merge 3 commits into from

Conversation

robberphex
Copy link
Contributor

@robberphex robberphex commented Sep 16, 2021

@robberphex
Copy link
Contributor Author

Now, the act of nerdctl in lima will be equivalent to docker:

# docker for desktop mac
$ /usr/local/bin/docker run -it --rm alpine:latest sh -x -c 'echo $HOME'
+ echo /root
/root
$ alias docker='limactl shell --raw default -- nerdctl'
$ docker run -it --rm alpine:latest sh -x -c 'echo $HOME'
+ echo /root
/root

Signed-off-by: luyanbo <robert.lyb@alibaba-inc.com>
Signed-off-by: luyanbo <robert.lyb@alibaba-inc.com>
Signed-off-by: luyanbo <robert.lyb@alibaba-inc.com>
@AkihiroSuda AkihiroSuda added this to the v0.7.0 milestone Sep 17, 2021
@AkihiroSuda AkihiroSuda marked this pull request as draft September 17, 2021 04:49
@@ -3,8 +3,7 @@ name: test
on:
push:
branches:
- master
- 'release/**'
- '*'
Copy link
Member

Choose a reason for hiding this comment

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

I do not think changes to the tests should be mixed into PRs that fix bugs or implement new features, but should be in their own PR.

I'm not sure why you would want to run tests on each push to any branch. I worry that this could use up some Github quota, so other tests may be queued later, instead of being executed right away. Not sure if that is a valid concern though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before creating PR, Github Action cannot be run at forked repo, this causes many PR with failed CI. Except that use branch release/* to create PR, it looks unreasonable.

The limit of branch to run CI, I think, it's not a good idea.

@jandubois
Copy link
Member

@robberphex

I just noticed that we did code review discussion in #251 and not here in the PR itself. Are you planning to adapt your PR to that feedback (create a separate limectl exec command)? Or do you want somebody else to finish it in a follow-up commit?

@AkihiroSuda
Copy link
Member

ping @robberphex

@AkihiroSuda AkihiroSuda removed this from the v0.7.0 milestone Oct 5, 2021
@robberphex
Copy link
Contributor Author

I'm working on create exec sub-command. Sorry for being have been on vacation.

@jandubois
Copy link
Member

Sorry for being have been on vacation.

No need to be sorry; I hope you enjoyed your time off! Welcome back! 😄

@jandubois
Copy link
Member

I'm working on create exec sub-command. Sorry for being have been on vacation.

@robberphex Are you still planning to work on this, or do you want somebody else to finish the nerdctl exec implementation?

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.

arguments of shell command doesn't support "raw"
3 participants