Skip to content

Conversation

@danielfeismann
Copy link
Member

resolves #278

@danielfeismann danielfeismann added the documentation Improvements or additions to documentation label Jul 21, 2022
@danielfeismann danielfeismann self-assigned this Jul 21, 2022
@sonarqubegithubprchecks

This comment has been minimized.

1 similar comment
@sonarqubegithubprchecks

This comment has been minimized.

@codecov
Copy link

codecov bot commented Jul 21, 2022

Codecov Report

Merging #279 (960a603) into main (e1913b5) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main     #279   +/-   ##
=========================================
  Coverage     66.14%   66.14%           
  Complexity      231      231           
=========================================
  Files            40       40           
  Lines          1208     1208           
  Branches        117      117           
=========================================
  Hits            799      799           
  Misses          376      376           
  Partials         33       33           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1913b5...960a603. Read the comment docs.

Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

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

Looks all good to me, thanks for this PR!

I noticed that not all units are registered with addUnit, thus parsing these quantities from Strings does not work. BUT, this shall be solved in a separate PR: #280 :)

@sebastian-peter sebastian-peter self-requested a review July 21, 2022 16:53
Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

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

Maybe I was a bit quick with my approval. I just noticed there are different definitions of "per unit": PU and p.u.
Does a standardized naming exist here as well? If not, we should at least provide some unified definition here.

@danielfeismann
Copy link
Member Author

Maybe I was a bit quick with my approval. I just noticed there are different definitions of "per unit": PU and p.u. Does a standardized naming exist here as well? If not, we should at least provide some unified definition here.

I did not find anything on this. Personally I would prefer p.u. and also this follows the rule of lowercase when it's not dedicated to a person (and isn't a liter :))

@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks

This comment has been minimized.

Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

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

I just noticed that an addition to the changelog is still missing :) Would be great if you could supply that

@sonarqubegithubprchecks

This comment has been minimized.

@sonarqubegithubprchecks
Copy link

Passed

Analysis Details

0 Issues

  • Bug0 Bugs
  • Vulnerability0 Vulnerabilities
  • Code Smell0 Code Smells

Coverage and Duplications

  • 100 percent coverage100.00% Coverage (64.70% Estimated after merge)
  • 3 percent duplication0.00% Duplicated Code (0.00% Estimated after merge)

Project ID: edu.ie3:utils

View in SonarQube

Copy link
Member

@sebastian-peter sebastian-peter left a comment

Choose a reason for hiding this comment

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

Thanks a lot!

@sebastian-peter sebastian-peter merged commit 5427d62 into main Jul 28, 2022
@sebastian-peter sebastian-peter deleted the df/#278_units_symbols_to_DIN branch July 28, 2022 11:34
@sebastian-peter sebastian-peter added this to the Version 2.0 milestone Jan 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unit symbols according to DIN 1301 Teil1

2 participants