Skip to content

[Improvement]: Remove space between QuantityValue value and unit#18313

Merged
kingjia90 merged 11 commits intopimcore:2026.xfrom
dolmit-tanel-paaro:improvement/remove-space-between-quantityvalue-value-and-unit
Mar 25, 2026
Merged

[Improvement]: Remove space between QuantityValue value and unit#18313
kingjia90 merged 11 commits intopimcore:2026.xfrom
dolmit-tanel-paaro:improvement/remove-space-between-quantityvalue-value-and-unit

Conversation

@dolmit-tanel-paaro
Copy link
Copy Markdown
Contributor

@dolmit-tanel-paaro dolmit-tanel-paaro commented Apr 15, 2025

Changes in this pull request

Resolves #18312

Additional info

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 15, 2025

Review Checklist

  • Target branch (11.5 for bug fixes, others 12.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

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Apr 15, 2025

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 SonarQube Cloud

@jdreesen jdreesen mentioned this pull request Apr 17, 2025
Merged
@kjkooistra-youwe
Copy link
Copy Markdown
Contributor

kjkooistra-youwe commented Apr 17, 2025

This may introduce a breaking change if people rely on this to split up the value and unit.

@dolmit-tanel-paaro
Copy link
Copy Markdown
Contributor Author

dolmit-tanel-paaro commented Apr 17, 2025 •
edited
Loading

Yeah, true, this will break exploding the string at the space character. But I personally think that's not a blocker - if you want to extract the number from the string, you can use regex.

@ghost ghost added the Pimcore:ToDo label Apr 28, 2025
@herbertroth herbertroth added this to the 13.0.0 milestone Sep 16, 2025
@kingjia90

This comment was marked as resolved.

Sign in to view
@kingjia90 kingjia90 self-assigned this Oct 9, 2025
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Oct 15, 2025

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 SonarQube Cloud

@kingjia90 kingjia90 mentioned this pull request Nov 5, 2025
Merged
Copilot AI review requested due to automatic review settings February 10, 2026 14:12
Copilot AI reviewed Feb 10, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the hardcoded space between QuantityValue numeric/text value and its unit abbreviation so spacing can be controlled externally (e.g., via translations), and documents the behavior change in upgrade notes (resolves #18312).

Changes:

  • Remove the " " separator in __toString() for QuantityValue and InputQuantityValue.
  • Remove the " " separator in admin version preview and QuantityValueRange CSV export formatting.
  • Add an upgrade note describing the spacing change.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
models/DataObject/Data/QuantityValue.php Removes hardcoded space before unit abbreviation in __toString().
models/DataObject/Data/InputQuantityValue.php Removes hardcoded space before translated unit abbreviation in __toString().
models/DataObject/ClassDefinition/Data/QuantityValueRange.php Removes hardcoded space before unit abbreviation in CSV export string.
models/DataObject/ClassDefinition/Data/AbstractQuantityValue.php Removes hardcoded space before unit abbreviation in version preview formatting.
doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md Adds upgrade note announcing the spacing change.

Copilot AI review requested due to automatic review settings March 25, 2026 16:22
Copilot AI reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

@kingjia90
Apply suggestions from code review
b3f73df 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 25, 2026 16:31
Copilot AI reviewed Mar 25, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

@kingjia90 kingjia90 merged commit 0f2adc5 into pimcore:2026.x Mar 25, 2026
14 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

@kingjia90 kingjia90

Labels

BC-BREAK Improvement Pimcore:ToDo

Projects

None yet

Milestone

2026.1.0

Development

Successfully merging this pull request may close these issues.

[Improvement]: Remove space between QuantityValue value and unit

7 participants

@dolmit-tanel-paaro @kjkooistra-youwe @kingjia90 @fashxp @herbertroth @astapc