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.

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

fix: Set s_binningValues as const #1485

Merged

Conversation

CarloVarni
Copy link
Collaborator

Setting s_binningValues as const as well. The Atlas Thread-check is complaining about this since it suspect this may be not thread safe.

@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #1485 (facbe51) into main (42b16ef) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1485   +/-   ##
=======================================
  Coverage   48.60%   48.60%           
=======================================
  Files         380      380           
  Lines       20595    20595           
  Branches     9433     9433           
=======================================
  Hits        10010    10010           
  Misses       4097     4097           
  Partials     6488     6488           
Impacted Files Coverage Δ
Core/include/Acts/Utilities/BinningType.hpp 66.66% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@CarloVarni CarloVarni changed the title Set s_binningValues as const fix: Set s_binningValues as const Sep 1, 2022
Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

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

thanks! looks good to me. I will leave it up to @paulgessinger to approve

@andiwand andiwand added this to the next milestone Sep 2, 2022
@paulgessinger paulgessinger added automerge backport develop/v19.x Backport this PR to the v19.x series labels Sep 2, 2022
@paulgessinger
Copy link
Member

CI bridge refused because @CarloVarni is not on the allowlist yet. I'm merging this manually.

@paulgessinger paulgessinger merged commit b172918 into acts-project:main Sep 2, 2022
@acts-project-service
Copy link
Collaborator

The backport to develop/v19.x failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-develop/v19.x develop/v19.x
# Navigate to the new working tree
cd .worktrees/backport-develop/v19.x
# Create a new branch
git switch --create backport-1485-to-develop/v19.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 b172918b0cb02d910faabc534e8db2069c27e78a
# Push it to GitHub
git push --set-upstream origin backport-1485-to-develop/v19.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-develop/v19.x

Then, create a pull request where the base branch is develop/v19.x and the compare/head branch is backport-1485-to-develop/v19.x.

@github-actions github-actions bot removed the automerge label Sep 2, 2022
@paulgessinger paulgessinger modified the milestones: next, v20.1.0 Sep 2, 2022
paulgessinger pushed a commit to paulgessinger/acts that referenced this pull request Sep 22, 2022
Setting `s_binningValues` as `const` as well. The Atlas Thread-check is complaining about this since it suspect this may be not thread safe.
paulgessinger added a commit that referenced this pull request Sep 22, 2022
#1547)

Setting `s_binningValues` as `const` as well. The Atlas Thread-check is
complaining about this since it suspect this may be not thread safe.

Co-authored-by: Carlo Varni <75478407+CarloVarni@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport develop/v19.x Backport this PR to the v19.x series
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants