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

native go modules support proposal (originally named v2 proposal) #70

Closed
6 of 11 tasks
Bobgy opened this issue Jun 19, 2021 · 14 comments
Closed
6 of 11 tasks

native go modules support proposal (originally named v2 proposal) #70

Bobgy opened this issue Jun 19, 2021 · 14 comments

Comments

@Bobgy
Copy link
Collaborator

Bobgy commented Jun 19, 2021

Updates

2022-03-12: #70 (comment) This will be released as v1.1 instead, because the breaking change might not affect anyone now (TL;DR there's only a regression for people still using GOPATH). Also renamed this issue to make the purpose clearer.

2021-1-11: #70 (comment) I sent out a bunch of PRs for merging changes back to go-licenses repo. The final PRs I'm sending now try to change as minimal as possible. There are no substantial command/arg changes and there are no significant behavior changes. The only major theme is using go modules information to get license URLs more stably than the v1 go-licenses tool.

Original Proposal

Hi everyone, I'm building a rewrite of go-licenses that I propose to be go-licenses/v2.
Latest version on my dev branch: https://github.com/Bobgy/go-licenses/blob/main/v2. No longer latest.
Built binary releases: https://github.com/Bobgy/go-licenses/releases.

I initially built it as a new tool, but after chatting with @wlynch, we agreed that it would better be v2 of this tool rather than a new brand.

The major changes are:

  • only supports go modules, it can get license URL for modules without a need for you to vendor your dependencies.
  • do not assume each package has a single license, v2 will scan all the files for each module to find licenses. Deferred for future considerations.

For more details, refer to the README.

The new tool already addresses many open issues:

Deprioritized features

and might help the following (but needs confirmation):

Roadmap

@Bobgy
Copy link
Collaborator Author

Bobgy commented Jun 19, 2021

Process to v2

Initial PR: #67.
However, based on #67 (review).

Unfortunately this is a very large change. I've left some initial comments as a first pass, but we'll likely need to do multiple iterations to resolve everything. Something that would greatly help me out for reviewing is if we could break this up into separate PRs. Here's some individual chunks I see that might be good places to break this apart:

I'll be sending several PRs to gradually upstream the new tool instead.
While addressing comments, I'll be reducing the difference with go-licenses v1 and try to make upgrade easier.

Here, I'm using this issue to track progress and welcome any feedback & code reviews.

@Bobgy
Copy link
Collaborator Author

Bobgy commented Jun 19, 2021

EDIT: Roadmap section moved to top comment.

@mythi
Copy link

mythi commented Oct 14, 2021

Suggest: Save command: compress sources

@Bobgy
Copy link
Collaborator Author

Bobgy commented Dec 31, 2021

I sent out a bunch of PRs which are minimal and focused on one thing, and a few more are ready (but I want to wait for the first PRs getting reviewed first).
The rough idea is starting from a copy of v1 go-licenses tool and gradually replace each command with the new implementation, so that we keep the interface and behavior backward compatible as much as possible.

@Bobgy
Copy link
Collaborator Author

Bobgy commented Jan 11, 2022

Reply to #93 (review)

@wlynch let's track general discussion in the main issue?

General question before I start reviewing these PRs in more details - are the changes to csv/check changes introducing any changes w.r.t input/outputs of the commands, or is this strictly an upgrade using module info?

No, there are no substantial changes to io.

There is a minor and optional output format change of adding a space after each comma to make it convenient to click the license URL in terminal/editor, do you have any concerns?

Also, git-url argument is removed, because it's no longer used. (But I am reconsidering this, because it may be useful for the main module.)

If there isn't anything that would break compatibility with existing clients, it might make sense to just merge these into v1. 👀
The bits I was primarily concerned with the original v2 proposal were the new commands and the different output format.

Well, the major breaking change is that we no longer support non go module projects. That's probably worth a v2. What do you think?

@Bobgy
Copy link
Collaborator Author

Bobgy commented Jan 14, 2022

Agreement today:

  • let's use major branch approach of reaching v2, because the current v2 is a natural continuation of v1 with some minor breaking changes. Existing PRs may still be easy to merge into v2.

AI:

  • @wlynch will tag a v1.0.0 release, so that people won't get a WIP v2 version accidentally in master branch
  • @Bobgy will move PRs to target master branch.

We will re-discuss at what point we want to bump to v2 as the development continues.

@Bobgy
Copy link
Collaborator Author

Bobgy commented Jan 14, 2022

@wlynch #95, #101 and #94 are ready for review : )

@Bobgy
Copy link
Collaborator Author

Bobgy commented Jan 20, 2022

Note, I created a github release in my fork in case anyone want to try the proposed new tool before above PRs are merged: https://github.com/Bobgy/go-licenses/releases/tag/v0.1.0-2022-01-20

EDIT: my compiled binary did not quite work.

@Bobgy
Copy link
Collaborator Author

Bobgy commented Feb 2, 2022

Created a new release that can be installed from source: https://github.com/Bobgy/go-licenses/releases/tag/v2.0.0-alpha.0. It's mostly ready to use and near backwards compatible.

To try my fork:

go get github.com/Bobgy/go-licenses/v2
go-licenses --help

@oxisto
Copy link

oxisto commented Feb 10, 2022

Is it somehow planned / possible to configure the check command, e.g. not only check for forbidden license but also for restricted? Or to configure the list of forbidden libraries? Or to at least print out the license type in the csv so I can grep for it? :)

@Bobgy
Copy link
Collaborator Author

Bobgy commented Mar 1, 2022

Hi @oxisto, it's not in the scope. You might want to file a separate issue for this feature request.

@oxisto
Copy link

oxisto commented Mar 1, 2022

Hi @oxisto, it's not in the scope. You might want to file a separate issue for this feature request.

got it. thanks for your answer!

@Bobgy
Copy link
Collaborator Author

Bobgy commented Mar 12, 2022

Update, based on discussion with @wlynch, we think that the breaking change for GOPATH might not be noticeable, because we don't know anyone still using GOPATH now. Therefore, we will consider releasing the next version as v1.1 instead. There won't be an actual v2.

Let us know if you use go-licenses and still use GOPATH, we can consider making changes in this proposal an actual v2 when needed.

@Bobgy Bobgy changed the title go-licenses/v2 proposal go-licenses native go modules support proposal (originally named v2 proposal) Mar 12, 2022
@Bobgy Bobgy changed the title go-licenses native go modules support proposal (originally named v2 proposal) native go modules support proposal (originally named v2 proposal) Mar 12, 2022
@Bobgy
Copy link
Collaborator Author

Bobgy commented Apr 11, 2022

The major changes have been released as v1.1.
Future improvements will be tracked individually in issues.

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

No branches or pull requests

3 participants