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

Rename to toolkit #2

Merged
merged 5 commits into from
Jan 10, 2025
Merged

Rename to toolkit #2

merged 5 commits into from
Jan 10, 2025

Conversation

tomonorman
Copy link
Collaborator

renames utils to toolkit

@tomonorman tomonorman requested review from ic, fabid and azazdeaz December 6, 2024 01:13
Copy link

@fabid fabid left a comment

Choose a reason for hiding this comment

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

Better name indeed

README.md Outdated

Designed to be used with `artefacts-utils` this package contains code available under the GPL license.
Designed to be used with `artefacts-toolkit` this package contains code available under the GPL license.
Copy link

Choose a reason for hiding this comment

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

Does it work standalone too, as a library on its own? Wondering whether there is transitivity of licence risk.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It does yes. and the README notes how to use standalone.

Perhaps we should just not include in artefacts-toolkit-rosbag and artefacts-toolkit (so no umbrella wrapping). I believe that would make it "unmodified" when used, and thus not affecting licensing of the cli (if we include it in that) (?)

Copy link

@ic ic left a comment

Choose a reason for hiding this comment

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

One question I cannot answer, but basically all good, thank you~

@tomonorman
Copy link
Collaborator Author

I have renamed the func here to extract_video so that there is no umbrella wrapping (And so it isnt used) in other packages

@tomonorman tomonorman merged commit e1e7961 into main Jan 10, 2025
@tomonorman tomonorman deleted the rename-to-toolkit branch January 10, 2025 02:13
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.

3 participants