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.

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

fix: let CsvVolumeLayer writer also write passive surfaces #891

Merged
merged 4 commits into from
Jul 22, 2021
Merged

fix: let CsvVolumeLayer writer also write passive surfaces #891

merged 4 commits into from
Jul 22, 2021

Conversation

asalzburger
Copy link
Contributor

This PR includes a fix to the CsvTrackingGeometry writer that did omit passive layer volumes.

@asalzburger asalzburger added Bug Something isn't working Component - Examples Affects the Examples module labels Jul 20, 2021
@asalzburger asalzburger added this to the next milestone Jul 20, 2021
@codecov
Copy link

codecov bot commented Jul 20, 2021 •
edited

Codecov Report

Merging #891 (f651f81) into main (4340ad2) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #891   +/-   ##
=======================================
  Coverage   48.66%   48.66%           
=======================================
  Files         331      331           
  Lines       17129    17129           
  Branches     8094     8094           
=======================================
  Hits         8336     8336           
  Misses       3087     3087           
  Partials     5706     5706

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 4340ad2...f651f81. Read the comment docs.

@asalzburger
Copy link
Contributor Author

Hey @robertlangenberg - as discussed yesterday, the changes look bigger in the file as they are.

  • the lambda function previously inlined has been refactored outside
  • the logic has been changed such that also passive layers are caught

robertlangenberg
robertlangenberg approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@robertlangenberg robertlangenberg left a comment

Choose a reason for hiding this comment

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

looks good to me

Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp Show resolved Hide resolved
Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp Outdated Show resolved Hide resolved
@asalzburger @robertlangenberg
Update Examples/Io/Csv/src/CsvTrackingGeometryWriter.cpp
f651f81 
Adressing PR comment.

Co-authored-by: robertlangenberg <56069170+robertlangenberg@users.noreply.github.com>
@asalzburger
Copy link
Contributor Author

This has been approved.

@asalzburger asalzburger merged commit 0ee97ff into acts-project:main Jul 22, 2021
@asalzburger asalzburger deleted the fix-csv-volume-writer-passive-layers branch July 22, 2021 04:59
@paulgessinger paulgessinger modified the milestones: next, v10.0.0 Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robertlangenberg robertlangenberg robertlangenberg approved these changes

Assignees
No one assigned
Labels
Bug Something isn't working Component - Examples Affects the Examples module
Projects
None yet
Milestone
v10.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@asalzburger @robertlangenberg @paulgessinger