-
Notifications
You must be signed in to change notification settings - Fork 3
Multiapplication Support #359
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
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.
Looks good, some minor comments though and also:
- Since I think the original goal of the ticket kinda changed its probs worth adding a full list of changes in the PR description
- I think you may need to regenerate mocks as you did end up refactoring the repository layer quite a bit.
- You will need to update
//go:generate mockgen -source=models.go -destination=mocks/models_mock.go -package=mocks //go:generate mockgen -source=repository_interfaces.go -destination=mocks/repositories_mock.go -package=mocks
- And then run
go generate ./...
at the top level in the backend directory
- You will need to update
Thanks for the meeting Brian. Looks great to me, just want there to be unit tests for multiple frontend repositories at once. |
Luckily this test did actually spot a bug where |
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.
That new test looks good to me. Great work Brian!
Generalising the CMS to enable other teams to utilise it. Improving user experience & security issues with sharing files in one large repository.
Update (rest of the specifics above are same):