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

added the docs directory with design report for kpm sparse checkout #334

Closed
wants to merge 36 commits into from

Conversation

d4v1d03
Copy link
Contributor

@d4v1d03 d4v1d03 commented May 24, 2024

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

  • N
  • Y

design doc for the issue #304

Signed-off-by: d4v1d03 <a_pandey1@ce.iitr.ac.in>
@coveralls
Copy link

coveralls commented May 24, 2024

Pull Request Test Coverage Report for Build 9358548895

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 43.549%

Totals Coverage Status
Change from base Build 9301223610: 0.0%
Covered Lines: 2474
Relevant Lines: 5681

💛 - Coveralls

@Peefy
Copy link
Contributor

Peefy commented May 24, 2024

@d4v1d03
Is this a design document related to PreTest? I personally don't think there are many useful parts. At least it should include

  • User interface/story: How users can add Git repository subdirectories as dependencies through the kcl command line, and what are the similarities and differences between its operation methods and other tools such as Cargo? What are the precautions for these.
  • The KCL package manager currently uses go getter to add Git dependencies. How do we integrate with go getter to add Git subdirectories.

cc @zong-zhe Could you please give more comments?

@d4v1d03
Copy link
Contributor Author

d4v1d03 commented May 24, 2024

I'll update it accordingly

@officialasishkumar
Copy link
Contributor

Hey @Peefy

other tools such as Cargo

Can you please elaborate on the tool in cargo which has similar functionality?

@Peefy
Copy link
Contributor

Peefy commented May 24, 2024

Hey @Peefy

other tools such as Cargo

Can you please elaborate on the tool in cargo which has similar functionality?

Cargo, go, helm, terraform and etc.

@Peefy
Copy link
Contributor

Peefy commented May 28, 2024

@d4v1d03 For developers, how they add the subfolder for git dependency through the KCL CLI? Could you please point it out in the design doc? cc @zong-zhe Cloud you please help review it?

@d4v1d03
Copy link
Contributor Author

d4v1d03 commented May 28, 2024

Could you pls point out why these tests are failing, is it because of low code cov? or anything else related to tests

@Peefy
Copy link
Contributor

Peefy commented May 28, 2024

Could you pls point out why these tests are failing, is it because of low code cov? or anything else related to tests

You've changed some files, so the CI failed.

@Peefy
Copy link
Contributor

Peefy commented May 28, 2024

Hello @d4v1d03 Please avoid directly modifying irrelevant code in the PR of the plan. The design looks good to me.

@d4v1d03
Copy link
Contributor Author

d4v1d03 commented May 28, 2024

yeah @Peefy , am really sorry, i'll take care for that in my next PR

pkg/package/modfile.go Outdated Show resolved Hide resolved
Considering the nginx-ingres module, on typing the command

```
kcl mod add https://github.com/kcl-lang/modules/tree/main/nginx-ingress /restrict-ingress-anotations
Copy link
Contributor

Choose a reason for hiding this comment

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

I just have a question, why don't you just merge these two parameters, something like this?

kcl mod add https://github.com/kcl-lang/modules/tree/main/nginx-ingress/restrict-ingress-anotations

I'm not sure what the problems will be behind such a merging, so you need to consider it or have a research.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done the changes, i did some research and gave thought to it but i don't think there won't be much of any problem. If any problems arise during implementation, i'll adjust accordingly.

version = "0.0.1"

[dependencies]
my_dependency = { git = "https://github.com/example/repo", subdirectory = "path/to/package" }
Copy link
Contributor

Choose a reason for hiding this comment

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

The problem here is the same as above.

[dependencies]
my_dependency = { git = "https://github.com/example/repo/path/to/package" }

Any problem like this ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made the changes

my_dependency = { git = "https://github.com/example/repo", subdirectory = "path/to/package" }
```

## 6. Integration and the use of go-getter to download the specific subdirectories
Copy link
Contributor

Choose a reason for hiding this comment

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

This chapter I think could need more details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll add some on my own, though can you still point out some specific details (if any) you're looking for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d4v1d03 and others added 14 commits May 31, 2024 02:09
Signed-off-by: d4v1d03 <a_pandey1@ce.iitr.ac.in>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <a_pandey1@ce.iitr.ac.in>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
…file to local system

Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <a_pandey1@ce.iitr.ac.in>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: zongz <zongzhe1024@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <a_pandey1@ce.iitr.ac.in>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
…and test cases

Signed-off-by: peefy <xpf6677@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Peefy and others added 15 commits May 31, 2024 02:09
Signed-off-by: peefy <xpf6677@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: peefy <xpf6677@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: peefy <xpf6677@163.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [contributor-assistant/github-action](https://github.com/contributor-assistant/github-action) from 2.3.0 to 2.4.0.
- [Release notes](https://github.com/contributor-assistant/github-action/releases)
- [Commits](contributor-assistant/github-action@v2.3.0...v2.4.0)

---
updated-dependencies:
- dependency-name: contributor-assistant/github-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [goreleaser/goreleaser-action](https://github.com/goreleaser/goreleaser-action) from 4 to 5.
- [Release notes](https://github.com/goreleaser/goreleaser-action/releases)
- [Commits](goreleaser/goreleaser-action@v4...v5)

---
updated-dependencies:
- dependency-name: goreleaser/goreleaser-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 2 to 4.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v2...v4)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 2 to 5.
- [Release notes](https://github.com/actions/setup-go/releases)
- [Commits](actions/setup-go@v2...v5)

---
updated-dependencies:
- dependency-name: actions/setup-go
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [github.com/onsi/ginkgo/v2](https://github.com/onsi/ginkgo) from 2.11.0 to 2.19.0.
- [Release notes](https://github.com/onsi/ginkgo/releases)
- [Changelog](https://github.com/onsi/ginkgo/blob/master/CHANGELOG.md)
- [Commits](onsi/ginkgo@v2.11.0...v2.19.0)

---
updated-dependencies:
- dependency-name: github.com/onsi/ginkgo/v2
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [github.com/go-git/go-git/v5](https://github.com/go-git/go-git) from 5.11.0 to 5.12.0.
- [Release notes](https://github.com/go-git/go-git/releases)
- [Commits](go-git/go-git@v5.11.0...v5.12.0)

---
updated-dependencies:
- dependency-name: github.com/go-git/go-git/v5
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Bumps [github.com/hashicorp/go-version](https://github.com/hashicorp/go-version) from 1.6.0 to 1.7.0.
- [Release notes](https://github.com/hashicorp/go-version/releases)
- [Changelog](https://github.com/hashicorp/go-version/blob/main/CHANGELOG.md)
- [Commits](hashicorp/go-version@v1.6.0...v1.7.0)

---
updated-dependencies:
- dependency-name: github.com/hashicorp/go-version
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <a_pandey1@ce.iitr.ac.in>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
This reverts commit 78e34fc.

Signed-off-by: d4v1d03 <amit08072005@gmail.com>
@d4v1d03 d4v1d03 force-pushed the main branch 2 times, most recently from 72d5758 to ab10bdc Compare May 30, 2024 20:58
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
@d4v1d03
Copy link
Contributor Author

d4v1d03 commented May 30, 2024

@Peefy , @zong-zhe please ignore the mess, just fixed my DCO, done the doc changes as per zong-zhe suggested.

d4v1d03 added 2 commits May 31, 2024 02:59
Signed-off-by: d4v1d03 <amit08072005@gmail.com>
.github/dependabot.yaml Outdated Show resolved Hide resolved
@zong-zhe
Copy link
Contributor

zong-zhe commented Jun 6, 2024

Hi @d4v1d03 😄

I've been looking at the cargo's design for this feature. I have some new findings to share with you. Hope this helpful.

I find this design comfortable to use. ❤️
It is here: https://doc.rust-lang.org/stable/cargo/reference/specifying-dependencies.html#renaming-dependencies-in-cargotoml

The cargo.toml looks like this:

[package]
name = "mypackage"
version = "0.0.1"

[dependencies]
bar = { git = "https://github.com/example/project", package = "foo" }

It will clone the git repo -> recursively look for all cargo.toml in the subdirectory -> find the package named foo -> load it.

I tried to use this feature on a project and found several advantages of this design.

  • Compared to adding a subdirectory to the URL, this way the user does not need to bother typing the URL. The URL of the subdirectory in the browser is not directly usable. 😅
Screenshot 2024-06-06 at 11 13 02
  • When a user decides to use a dependency, he should probably know the package name of the dependency. 😆

@d4v1d03
Copy link
Contributor Author

d4v1d03 commented Jun 6, 2024

Hey @zong-zhe , yes my research over the design i proposed was inspired from cargo's package managemnet. The same reason i was clarifying regarding the recursive check on all kcl.mod files. Thanks a lot for clarification over this.😊

@zong-zhe zong-zhe closed this Jul 9, 2024
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.

Enhancement: kpm add command displaying optimization
5 participants