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

feat: support for automatic update KCL dependencies on kcl.mod file save #1118

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

octonawish-akcodes
Copy link
Contributor

@octonawish-akcodes octonawish-akcodes commented Mar 8, 2024

  • Added a new function watch_kcl_mod to monitor changes to the kcl.mod file and trigger dependency updates accordingly.

  • Added a separate kcl_mod_file_track function that can be called from the LSP implementation(yet to be explore)

I want to know what target folders can be used for the calling of this kcl_mod_file_track

Related Issue #998

cc @Peefy @zong-zhe @He1pa

@Peefy Peefy added the lsp label Mar 8, 2024
@Peefy Peefy requested review from He1pa and zong-zhe March 8, 2024 05:44
@Peefy Peefy added this to the v0.9.0 Release milestone Mar 8, 2024
@Peefy
Copy link
Contributor

Peefy commented Mar 8, 2024

I think from a technical architecture perspective, this should be a more universal build system that can be used not only to monitor changes in kcl.mod files, but also to monitor changes in other files in the KCL ecosystem (*.k even if *.yaml, *.json). What do you think of these things when it comes to creating the kcl build -- watch command in the end? cc @He1pa @octonawish-akcodes

@octonawish-akcodes
Copy link
Contributor Author

What is the scope of this kcl build -- watch, I mean what usability will it provide to the KCL user can you elaborate a bit?

@Peefy
Copy link
Contributor

Peefy commented Mar 8, 2024

In summary, kcl build --watch maybe KCL compiler's watch mode, which helps developers to compile KCL code in real-time and receive prompt feedback. This is particularly useful for rapid iteration during development and continuous integration.

That is to say, I believe that the PR should not only watch the kcl.mod file, as such code does not have good scalability. It should be a part of the entire watch system.

kclvm/driver/src/kpm.rs Outdated Show resolved Hide resolved
@coveralls
Copy link
Collaborator

coveralls commented Mar 8, 2024

Pull Request Test Coverage Report for Build 8243675034

Details

  • 0 of 66 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.08%) to 71.828%

Changes Missing Coverage Covered Lines Changed/Added Lines %
kclvm/driver/src/kpm.rs 0 66 0.0%
Totals Coverage Status
Change from base Build 8242468454: -0.08%
Covered Lines: 44677
Relevant Lines: 62200

💛 - Coveralls

@octonawish-akcodes
Copy link
Contributor Author

In summary, kcl build --watch maybe KCL compiler's watch mode, which helps developers to compile KCL code in real-time and receive prompt feedback. This is particularly useful for rapid iteration during development and continuous integration.

That is to say, I believe that the PR should not only watch the kcl.mod file, as such code does not have good scalability. It should be a part of the entire watch system.

hmm, that's a good suggestion tho, we can try to add that in parts like first the flag implementation and then the corresponding implementation of functionalities. Not sure about the real-time running of kcl package(did you meant after every change in the file kcl run should get triggered?)

@Peefy
Copy link
Contributor

Peefy commented Mar 8, 2024

I suggest first providing the tech design of the Watch System, including an explanation of its scalability, how to extend kcl. mod files, how to access more file types such as JSON, YAML, and the watch system APIs, etc., without the need to first implement flags.

@octonawish-akcodes
Copy link
Contributor Author

I suggest first providing the tech design of the Watch System, including an explanation of its scalability, how to extend kcl. mod files, how to access more file types such as JSON, YAML, and the watch system APIs, etc., without the need to first implement flags.

Should I start a new discussion in the discussion topics or add a new design in the docs and raise a PR?

@Peefy
Copy link
Contributor

Peefy commented Mar 8, 2024

I suggest first providing the tech design of the Watch System, including an explanation of its scalability, how to extend kcl. mod files, how to access more file types such as JSON, YAML, and the watch system APIs, etc., without the need to first implement flags.

Should I start a new discussion in the discussion topics or add a new design in the docs and raise a PR?

Yes.

kclvm/driver/src/kpm.rs Outdated Show resolved Hide resolved
kclvm/driver/src/kpm.rs Outdated Show resolved Hide resolved
kclvm/driver/src/kpm.rs Outdated Show resolved Hide resolved
kclvm/driver/src/kpm.rs Outdated Show resolved Hide resolved
Signed-off-by: Abhishek Kumar <abhishek22512@gmail.com>
@Peefy
Copy link
Contributor

Peefy commented Mar 12, 2024

I will merge the PR, this is just a simple first step. The subsequent design and implementation need to be combined with the watch system, file system, and language server to clarify the technical architecture. cc @zong-zhe @He1pa

@Peefy Peefy merged commit 3a83edf into kcl-lang:main Mar 12, 2024
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants