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

The MiniPaint integration #817

Merged
merged 31 commits into from
Jan 14, 2025
Merged

The MiniPaint integration #817

merged 31 commits into from
Jan 14, 2025

Conversation

ValeriaMaltseva
Copy link
Contributor

Task: #740

@ValeriaMaltseva ValeriaMaltseva marked this pull request as draft December 11, 2024 10:05
@ValeriaMaltseva ValeriaMaltseva changed the base branch from 1.x to minipaint January 8, 2025 11:32
@ValeriaMaltseva ValeriaMaltseva marked this pull request as ready for review January 8, 2025 15:02
Copy link
Collaborator

@vin0401 vin0401 left a comment

Choose a reason for hiding this comment

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

I saw that you introduced a new folder called services.
I'm not sure what the purpose/meaning of this folder is.

Could elaborate a bit on that?
Right now, we already have a lib folder maybe that one would also be suitable not sure.

For me services are more like useful classes/objects that are provided via our dependency injection container. So I'm not sure if the name is really suitable in that case.

@ValeriaMaltseva ValeriaMaltseva changed the base branch from minipaint to 1.x January 9, 2025 16:00
@markus-moser
Copy link
Contributor

@ValeriaMaltseva Only found one small topic (see my comment). Other then that I tested it a bit and everything seems to be fine. We can merge it after checking my comment. 👍

@ValeriaMaltseva ValeriaMaltseva merged commit 6e2eca9 into 1.x Jan 14, 2025
1 check passed
@ValeriaMaltseva ValeriaMaltseva deleted the 740-minipaint branch January 14, 2025 10:01
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants