-
-
Notifications
You must be signed in to change notification settings - Fork 3
Some cleanups to the codebase #293
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
Conversation
Tests have been moved in the corresponding internal package.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review @per1234! Are we using this package somewhere? I could not find any reference to it. If it's not used I'd prefer to remove it (even if it's technically a breaking change, I'm pretty sure nobody outside Arduino is using it). If we are using I'd like to replace it with a package in the |
Yes, it is used here: |
It doesn't make sense to move it to the |
I see. What about moving I think this would give us some long-term advantages:
|
Sounds good. |
2484717
to
4a522c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Don't forget to open a PR to change this workflow: https://github.com/arduino/library-registry/blob/production/.github/workflows/assets/validate-registry/main.go
As pointed out by @per1234
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Cristian!
io/ioutils
with the equivalent callslibraries
package at the top level, and merged the tests with the existinginternal/libraries
package.