-
Notifications
You must be signed in to change notification settings - Fork 11.1k
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
regression: Move WorkspaceCredentials models to CE #33657
Conversation
Looks like this PR is ready to merge! 🎉 |
|
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.
What happens if I don't have the license but then I install it?
I don't see much problem with a model being public, what we should be concerned about are the controllers/logic |
Good Question, I have no idea, maybe @Gustrb can help us. |
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #33657 +/- ##
========================================
Coverage 74.85% 74.85%
========================================
Files 470 470
Lines 20744 20744
Branches 5295 5295
========================================
Hits 15528 15528
Misses 4595 4595
Partials 621 621
Flags with carried forward coverage won't be shown. Click here to find out more. |
2nd vote for making the model CE. Nothing on the model needs to be EE as long as whatever is calling it is on EE. |
3rd vote on moving the model to CE. I'll take care of this. |
Yea I think the best approach would be to make the models public and avoid having to do weird things such as migrating on license change |
b69124c
to
607996c
Compare
@ggazzo Moved the models to CE |
Can you update the PR title as well? We're not skipping migration anymore |
Proposed changes (including videos or screenshots)
Migration 316 breaks community edition, the migration tries to use some models which are only present in EE, which crashes the server.
Fixing it by moving the required models to CE
Issue(s)
Steps to test or reproduce
Further comments