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

chore: expose manifest reading and jsonwall as packages #182

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

letFunny
Copy link
Collaborator

@letFunny letFunny commented Dec 13, 2024

  • Have you signed the CLA?

References:

Relevant excerpts:

/pkg
Library code that's ok to use by external applications (e.g., /pkg/mypubliclib). Other projects will import these libraries > expecting them to work, so think twice before you put something here
link

@letFunny letFunny changed the title chose: expose manifest reading and jsonwall as packages chore: expose manifest reading and jsonwall as packages Dec 13, 2024
@letFunny letFunny added the Polish Refactorings, etc label Dec 13, 2024
@letFunny letFunny mentioned this pull request Dec 13, 2024
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

We'll likely need to add the new licensing headers to each one of these source files

@letFunny letFunny marked this pull request as ready for review January 7, 2025 10:22
Copy link
Collaborator

@cjdcordeiro cjdcordeiro left a comment

Choose a reason for hiding this comment

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

thanks for adding the licensing headers. they lgtm

@cjdcordeiro
Copy link
Collaborator

Note

pls ignore the failed RTD check. I've enabled the automatic generation of PR docs, which will start passing once #185 is merged

@letFunny letFunny requested a review from cjdcordeiro January 7, 2025 17:55
"io"
"slices"

"github.com/canonical/chisel/internal/setup"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given that chisel/internal is under AGPL, is this ok?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Honestly, I am not sure. I have to read more about licensing, but it looks like it will not be okay, we would have to extract the common components into a Apache-2.0 package that is imported by both the AGPL and this one, but I am not really sure.


. "gopkg.in/check.v1"

"github.com/canonical/chisel/internal/testutil"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same comment here as above

@letFunny letFunny added the Blocked Waiting for something external label Jan 8, 2025
@letFunny
Copy link
Collaborator Author

letFunny commented Jan 8, 2025

I am blocking this until we solve all the questions regarding copyright and licensing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocked Waiting for something external Polish Refactorings, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants