search

go-task / task

A task runner / simpler Make alternative written in Go
https://taskfile.dev
MIT License
11.56k stars 616 forks source link

refactor: use giturl pkg instead of chainguard-dev/git-urls #1918

Closed alexandear closed 2 weeks ago

alexandear commented 2 weeks ago

This PR introduces the giturl package with a single Parse function to parse Git URLs. No need to import the whole package just for the one function.

The giturl package is a refactored copy of https://github.com/chainguard-dev/git-urls with unexported methods and extended tests.

This PR follows the Go proverb "A little copying is better than a little dependency".

Relates to #1917

andreynering commented 2 weeks ago

Hi @alexandear,

I understand the principle of reducing dependencies, and I generally agree with it, but it is a case-by-case decision.

In this case, we're adding 500+ lines of code (including tests) to the codebase to avoid a dependency. I'm not so sure it is worth it. In this case, it might be better to just keep the dependency, specially considering it does not import other packages, so we're talking about a single smallish package here.

pd93 commented 2 weeks ago

@andreynering Agreed. There is nothing wrong with the dependency IMO.

alexandear commented 2 weeks ago

Thank you for sharing your perspective. I understand your concerns about introducing 115 lines of code just to eliminate a single dependency. It's indeed important to weigh the benefits and potential maintenance overhead.

Removing dependencies can bring long-term benefits, such as reducing the risk of breaking changes and improving security because we fully control the package.

I've also included some articles that could provide additional insight on when it’s appropriate to minimize dependencies and when it might be acceptable to keep them:

It's ultimately up to you whether to accept this PR or not. Close the PR if it's not needed.

pd93 commented 2 weeks ago

Thanks for the PR. IMO most of the benefits of the principle you describe do not apply here so I'm going to close this.

An additional note. If we were to include a copy of this code in Task, it would be required that we extend our licence to cover the one in used in https://github.com/chainguard-dev/git-urls. We cannot simply take the code without crediting the author as per the MIT license.