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

Add pre-commit hook to prevent references to kolibri-common package in published packages #12891

Conversation

rtibbles
Copy link
Member

Summary

  • Most of our packages in the packages folder are, or will be, published to npm
  • The kolibri-common package is not, and probably never will be, as it is explicitly a place to put things we do not intend to support as a public API
  • During the core API refactor work, I failed to properly disentangle these two things
  • This adds a pre-commit hook to prevent this happening again, and fixes all instances where this had happened.

References

Cleanup from #12722

Reviewer guidance

Anything being put into the core API that shouldn't be? I preferred defining APIResources either in internal modules or inline in code rather than add them to public API for now.

@github-actions github-actions bot added DEV: renderers HTML5 apps, videos, exercises, etc. APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) DEV: frontend DEV: tools Internal tooling for development labels Nov 27, 2024
@rtibbles rtibbles force-pushed the none_of_your_common_ilk_here_please branch 2 times, most recently from 805c2e5 to 7e08249 Compare December 2, 2024 17:24
@rtibbles rtibbles marked this pull request as draft December 2, 2024 17:31
@rtibbles
Copy link
Member Author

rtibbles commented Dec 2, 2024

Set to draft as I continue to sort out minor issues.

@rtibbles rtibbles force-pushed the none_of_your_common_ilk_here_please branch from 7e08249 to d2f6bb4 Compare December 2, 2024 18:05
@rtibbles rtibbles marked this pull request as ready for review December 2, 2024 19:29
Copy link
Member

@nucleogenesis nucleogenesis left a comment

Choose a reason for hiding this comment

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

Gave this a look and everything looks good to me. Lots of stuff covered here so I suspect if the linting and tests pass things should be good to go here.

…n published packages.

Add pre-commit hook to rebuild kolibri package.
Fix all flagged issues.
@rtibbles rtibbles force-pushed the none_of_your_common_ilk_here_please branch from d2f6bb4 to 9f0dec1 Compare December 13, 2024 19:09
@rtibbles rtibbles merged commit 504fc58 into learningequality:develop Dec 13, 2024
37 checks passed
@rtibbles rtibbles deleted the none_of_your_common_ilk_here_please branch December 13, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Device Re: Device App (content import/export, facility-syncing, user permissions, etc.) APP: Facility Re: Facility App (user/class management, facility settings, csv import/export, etc.) APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: Setup Wizard Re: Setup Wizard (facility import, superuser creation, settings, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend DEV: renderers HTML5 apps, videos, exercises, etc. DEV: tools Internal tooling for development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants