Skip to content

Areablock: Accept array for setDataFromResource() #16773

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Sep 4, 2024
Merged

Areablock: Accept array for setDataFromResource() #16773

merged 8 commits into from
Sep 4, 2024

Conversation

blankse
Copy link
Contributor

@blankse blankse commented Mar 13, 2024 •
edited by kingjia90
Loading

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 13, 2024 •
edited by mattamon
Loading

Review Checklist

  • Target branch (11.2 for bug fixes, others 11.x)
  • Tests (if it's testable code, there should be a test for it - get help)
  • Docs (every functionality needs to be documented, see here)
  • Migration incl. install.sql (e.g. if the database schema changes, ...)
  • Upgrade notes (deprecations, important information, migration hints, ...)
  • Label
  • Milestone

@kingjia90 kingjia90 mentioned this pull request Mar 13, 2024
Closed
mattamon
mattamon reviewed Mar 29, 2024
View reviewed changes
@blankse blankse force-pushed the areablock_respect_array_data branch from df84246 to ce041a5 Compare April 1, 2024 12:53
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Apr 1, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@herbertroth herbertroth changed the base branch from 11.2 to 11.3 July 2, 2024 12:22
mattamon
mattamon suggested changes Aug 26, 2024
View reviewed changes
Copy link
Contributor

@mattamon mattamon left a comment

Choose a reason for hiding this comment

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

Seems like a lot of code duplication for me.

Could we not add this functionality in a trait or somewhere in the base class?

@mattamon mattamon self-assigned this Aug 26, 2024
@mattamon mattamon added this to the 11.3.3 milestone Sep 4, 2024
@mattamon
Copy link
Contributor

mattamon commented Sep 4, 2024

Seems like a lot of code duplication for me.

Could we not add this functionality in a trait or somewhere in the base class?

@blankse could you please take care of the code duplication? :)

@blankse blankse force-pushed the areablock_respect_array_data branch from 857ecac to 00bd697 Compare September 4, 2024 07:49
@blankse
Copy link
Contributor Author

blankse commented Sep 4, 2024

@mattamon Done with e6902fa

mattamon
mattamon reviewed Sep 4, 2024
View reviewed changes
@blankse blankse force-pushed the areablock_respect_array_data branch from e35038d to ad70c75 Compare September 4, 2024 08:43
@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Sep 4, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@mattamon mattamon added the Bug label Sep 4, 2024
mattamon
mattamon approved these changes Sep 4, 2024
View reviewed changes
Copy link
Contributor

@mattamon mattamon left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you as always for your contribution!

@mattamon mattamon merged commit 9ef66a1 into pimcore:11.3 Sep 4, 2024
19 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Sep 4, 2024
@blankse blankse deleted the areablock_respect_array_data branch September 4, 2024 09:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
1 more reviewer

@mattamon mattamon mattamon approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@mattamon mattamon

Labels
Bug Improvement Pimcore:ToDo
Projects
None yet
Milestone
11.3.3
Development

Successfully merging this pull request may close these issues.
3 participants
@blankse @mattamon @kingjia90