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 environment variables documentation #2526

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

SmartManoj
Copy link
Contributor

@SmartManoj SmartManoj commented Aug 2, 2024

Created a new PR to avoid the blockage of #2455

Add documentation for environment variables used in Lima.

  • New Documentation Page: Add website/content/en/docs/config/environment-variables.md to document environment variables LIMA_INSTANCE, LIMA_SHELL, LIMA_WORKDIR, LIMACTL, and LIMA_USERNET_RESOLVE_IP_ADDRESS_TIMEOUT.
  • Update Configuration Guide: Modify website/content/en/docs/config/_index.md to include a link to the new 'Environment Variables' page.
  • Update README: Modify README.md to include a link to the new 'Environment Variables' page.
  • Add Comments in Scripts:
    • cmd/lima: Add comments documenting LIMA_INSTANCE, LIMA_SHELL, LIMA_WORKDIR, and LIMACTL.
    • cmd/lima.bat: Add comments documenting LIMA_INSTANCE and LIMACTL.
    • cmd/docker.lima: Add comments documenting LIMA_INSTANCE.
    • cmd/kubectl.lima: Add comments documenting LIMA_INSTANCE.
    • cmd/limactl/usernet.go: Add comments documenting LIMA_USERNET_RESOLVE_IP_ADDRESS_TIMEOUT.

For more details, open the Copilot Workspace session.

README.md Outdated Show resolved Hide resolved
cmd/docker.lima Outdated Show resolved Hide resolved
@jandubois
Copy link
Member

Also all the file mode changes are incorrect; these files are supposed to be executable.

@jandubois
Copy link
Member

I've not looked at the most recent "fix erroneous change" commit, but the file modes seem still wrong.

Note that when you use an AI tool, it is your responsibility to review the generated code and docs for obvious errors and not leave it to the PR reviewer to clean up after the tool!

And while I don't think it is an issue here, it is also your responsibility to make sure you comply with the Generative AI Policy | Linux Foundation.

@SmartManoj
Copy link
Contributor Author

but the file modes seem still wrong.

For #2455 (comment), I amended with the first commit in Windows, and the diff shows file modes are changed. So, I thought Windows reverted the file modes but it seems it's the output of the first diff.

@jandubois
Copy link
Member

the diff shows file modes are changed

The file mode for cmd/docker.lima is still wrong:

$ g log --summary -2
commit 93ac0cbc28ce49df08a46b95d7c0bb5581654488 (HEAD -> add-environment-variables)
Author: மனோஜ்குமார் பழனிச்சாமி <[email protected]>
Date:   Sat Aug 3 08:15:20 2024 +0530

    revert file modes

    Signed-off-by: மனோஜ்குமார் பழனிச்சாமி <[email protected]>

 mode change 100644 => 100755 cmd/kubectl.lima
 mode change 100644 => 100755 cmd/lima
 mode change 100644 => 100755 cmd/lima.bat

commit 1ae62d692209495f7839f2ab35a34309fa37aadf
Author: மனோஜ்குமார் பழனிச்சாமி <[email protected]>
Date:   Fri Aug 2 10:47:17 2024 +0530

    fix erroneous change

    Signed-off-by: மனோஜ்குமார் பழனிச்சாமி <[email protected]>

 mode change 100755 => 100644 cmd/docker.lima
 mode change 100755 => 100644 cmd/kubectl.lima
 mode change 100755 => 100644 cmd/lima
 mode change 100755 => 100644 cmd/lima.bat
 create mode 100644 website/content/en/docs/config/environment-variables.md

Also please squash commits and create a meaningful commit message!

@SmartManoj
Copy link
Contributor Author

$ git log --summary -1
commit 7118626d9b1d346c18c8a2b4f96ae25787a3b11e (HEAD -> add-environment-variables, origin/add-environment-variables)
Author: மனோஜ்குமார் பழனிச்சாமி <[email protected]>
Date:   Fri Aug 2 10:47:17 2024 +0530

    Add environment variables documentation

    Signed-off-by: மனோஜ்குமார் பழனிச்சாமி <[email protected]>

 create mode 100644 website/content/en/docs/config/environment-variables.md

export LIMA_SHELL=/bin/bash
lima
```
- **Used in**: `cmd/lima`, `cmd/limactl/shell.go`
Copy link
Member

@AkihiroSuda AkihiroSuda Aug 12, 2024

Choose a reason for hiding this comment

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

I don't think this env var is used in cmd/limactl/shell.go.
Even if it was used, such an implementation detail would not need to be documented here.

Copy link
Member

Choose a reason for hiding this comment

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

The same applies to other *.go source files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

.go files are mentioned here only. This PR is created mainly for this environment var only.

Copy link
Member

Choose a reason for hiding this comment

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

The implementation details shouldn't be mentioned in user docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which line are you referring to?

Copy link
Member

Choose a reason for hiding this comment

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

usernet.go

Signed-off-by: மனோஜ்குமார் பழனிச்சாமி <[email protected]>
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda added this to the v1.0 milestone Oct 16, 2024
@AkihiroSuda AkihiroSuda added the documentation Improvements or additions to documentation label Oct 16, 2024
@AkihiroSuda AkihiroSuda merged commit c8279f7 into lima-vm:master Oct 16, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants