Skip to content

[core] AddBefore/After/At option variant, as with Add/First/Last #17846

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

Closed
wants to merge 1 commit into from

Conversation

ferdymercury
Copy link
Collaborator

@ferdymercury ferdymercury commented Feb 28, 2025

This Pull request:

Changes or fixes:

Fixes https://its.cern.ch/jira/browse/ROOT-9288 (the first part)

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Copy link

Test Results

    18 files      18 suites   4d 11h 25m 49s ⏱️
 2 719 tests  2 719 ✅ 0 💤 0 ❌
47 253 runs  47 253 ✅ 0 💤 0 ❌

Results for commit 7132666.

@silverweed
Copy link
Contributor

Do we really want to add support for a class that users are not supposed to ever use directly anymore? Is there a compelling current use case that requires this?

@ferdymercury
Copy link
Collaborator Author

Do we really want to add support for a class that users are not supposed to ever use directly anymore? Is there a compelling current use case that requires this?

Maybe it's better to ask the reporter of the JIRA ticket, who requested it, if he really needs it.
I am happy either way, if not, in that case https://its.cern.ch/jira/browse/ROOT-9288 can be closed as won't fix.

@silverweed
Copy link
Contributor

Let's wait for @dpiparo and @pcanal's opinion on this.
The jira ticket is almost 7 years old at this point, so I doubt it's still relevant (but if we manage to contact the user we can hear what they have to say)

@dpiparo
Copy link
Member

dpiparo commented Apr 7, 2025

Even if this is a legacy class, since this nice work has been done, I would be in favour of merging and finally address the JIRA item.

@dpiparo
Copy link
Member

dpiparo commented Apr 7, 2025

@pcanal ?

@dpiparo
Copy link
Member

dpiparo commented Apr 9, 2025

Are you suggesting to close as "won't fix" because the cost-benefit ratio of the full solution would not be too favourable?

@ferdymercury
Copy link
Collaborator Author

Moved into #18353

@ferdymercury ferdymercury deleted the addopt branch April 10, 2025 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants