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

Pdu surface normal #1068

Merged
merged 15 commits into from
Oct 31, 2023
Merged

Pdu surface normal #1068

merged 15 commits into from
Oct 31, 2023

Conversation

chchatte92
Copy link
Member

@chchatte92 chchatte92 commented Oct 10, 2023 •
edited
Loading

Briefly, what does this PR introduce?

Added new methods inside richgeo/IrtGeoDRICH to extract sensor normalZ

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No

Does this PR change default behavior?

No

@chchatte92 chchatte92 requested a review from c-dilks October 10, 2023 22:07
@github-actions github-actions bot added topic: calorimetry relates to calorimetry topic: tracking Relates to tracking reconstruction topic: infrastructure labels Oct 10, 2023
@chchatte92 chchatte92 added topic: PID Relates to PID reconstruction and removed topic: calorimetry relates to calorimetry topic: tracking Relates to tracking reconstruction topic: infrastructure labels Oct 10, 2023
@c-dilks
Copy link
Member

c-dilks commented Oct 10, 2023 •
edited
Loading

Rebase this:

git fetch origin
git rebase --onto origin/main e8415e9

then force push

@github-actions github-actions bot added topic: calorimetry relates to calorimetry topic: tracking Relates to tracking reconstruction topic: infrastructure and removed topic: PID Relates to PID reconstruction labels Oct 10, 2023
@github-actions github-actions bot removed topic: calorimetry relates to calorimetry topic: tracking Relates to tracking reconstruction topic: infrastructure labels Oct 10, 2023
@chchatte92
Copy link
Member Author

Rebase this:

git fetch origin
git rebase --onto origin/main e8415e9

then force push

Done!

@pre-commit-ci pre-commit-ci bot temporarily deployed to github-pages October 10, 2023 23:18 Inactive
@pre-commit-ci pre-commit-ci bot temporarily deployed to github-pages October 11, 2023 18:53 Inactive
@chchatte92 chchatte92 marked this pull request as ready for review October 20, 2023 14:48
@chchatte92 chchatte92 temporarily deployed to github-pages October 20, 2023 17:32 — with GitHub Actions Inactive
c-dilks
c-dilks requested changes Oct 20, 2023
View reviewed changes
src/services/geometry/richgeo/IrtGeoDRICH.cc Outdated Show resolved Hide resolved
src/services/geometry/richgeo/IrtGeoDRICH.h Outdated Show resolved Hide resolved
src/services/geometry/richgeo/IrtGeoDRICH.cc Outdated Show resolved Hide resolved
src/services/geometry/richgeo/RichGeo.h Outdated Show resolved Hide resolved
chchatte92
chchatte92 commented Oct 23, 2023
View reviewed changes
Copy link
Member Author

@chchatte92 chchatte92 left a comment

Choose a reason for hiding this comment

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

Changed

@github-actions github-actions bot added topic: tracking Relates to tracking reconstruction topic: PID Relates to PID reconstruction topic: infrastructure labels Oct 25, 2023
@github-actions github-actions bot removed topic: tracking Relates to tracking reconstruction topic: PID Relates to PID reconstruction topic: infrastructure labels Oct 25, 2023
@chchatte92 chchatte92 temporarily deployed to github-pages October 25, 2023 14:09 — with GitHub Actions Inactive
@chchatte92
Copy link
Member Author

You'll need to apply the iwyu patch that causes the failed check. We started requiring that, for now on a pr-by-pr basis while we implement it treewide.

Hi @wdconinc , I have the main branch merged to this branch, git pulled and made sure it compiles in my local branch; but still here the iwyu check fails. What should I do?

@chchatte92 chchatte92 temporarily deployed to github-pages October 31, 2023 12:39 — with GitHub Actions Inactive
@chchatte92
Copy link
Member Author

@wdconinc and @c-dilks, seems like there are no more conflicts and failures. Shall we proceed to merge?

@wdconinc
Copy link
Contributor

@c-dilks Your changes-requested review is the only thing left blocking. Can you review, and approve if ok?

@wdconinc wdconinc enabled auto-merge October 31, 2023 21:44
@wdconinc wdconinc added this pull request to the merge queue Oct 31, 2023
@wdconinc wdconinc temporarily deployed to github-pages October 31, 2023 22:41 — with GitHub Actions Inactive
Merged via the queue into main with commit 0163cdb Oct 31, 2023
74 checks passed
@wdconinc wdconinc deleted the pdu-surface-normal branch October 31, 2023 23:12
@c-dilks c-dilks mentioned this pull request Nov 1, 2023
Draft
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@c-dilks c-dilks c-dilks approved these changes

@wdconinc wdconinc wdconinc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chchatte92 @c-dilks @wdconinc @veprbl