Skip to content

Unify file and dir names; integrate ls-lint #2476

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

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

alexandear
Copy link
Member

@alexandear alexandear commented Jul 9, 2024

The PR:

  • renames Go files in the cmd to match kebab-case for consistency;
  • renames pkg/networks/usernet/UDPFileConn.go to pkg/networks/usernet/udpfileconn.go
  • renames directories in website/content to be lowercase;
  • integrates ls-lint to ensure proper file and directory names in the future.

@alexandear alexandear added github_actions Pull requests that update GitHub Actions code and removed github_actions Pull requests that update GitHub Actions code labels Jul 9, 2024
@afbjorklund
Copy link
Member

afbjorklund commented Jul 9, 2024

I always found it annoying that the links and the files don't match, in the "website"

https://lima-vm.io/docs/examples/ -> website/content/en/docs/Examples

@jandubois
Copy link
Member

I'm not sure about using snake_case for the cmd/*.go files. I think the filename should match the subcommand, and it is limactl factory-reset and not limactl factory_reset.

So I would rather rename show_ssh.goshow-ssh.go instead.

But in the end it is just personal preference.

Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

I don't understand why factory-reset.go was renamed to factory-reset_test.go; it is not a test file.

That also breaks building the app:

undefined: newFactoryResetCommand

@alexandear alexandear marked this pull request as draft July 10, 2024 13:35
@alexandear alexandear changed the title Unify filenames; ensure fs names with ls-lint Unify file and dir names; integrate ls-lint Jul 10, 2024
@alexandear alexandear requested a review from jandubois July 10, 2024 14:34
@alexandear
Copy link
Member Author

I always found it annoying that the links and the files don't match, in the "website"

Renamed all directories to lowercase.

So I would rather rename show_ssh.go → show-ssh.go instead.

Changed.

I don't understand why factory-reset.go was renamed to factory-reset_test.go; it is not a test file.

Fixed. It's my fault.

@alexandear alexandear marked this pull request as ready for review July 10, 2024 14:35
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Since this is largely based on personal preferences, I'll leave the merging to somebody else from @lima-vm/maintainers to give them a chance to object to it.

@AkihiroSuda AkihiroSuda added this to the v1.0 (tentative) milestone Jul 16, 2024
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 merged commit aaf8b9c into lima-vm:master Jul 16, 2024
27 checks passed
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.

4 participants