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
C++: Remove getLocation from Container. #14337
base: main
Are you sure you want to change the base?
Conversation
|
Is it actually safe to just remove these, or do we need to go through a deprecation period? |
Good question. I just wanted to put up the diff for suggestion. The PR actually removes a whole bunch of other predicates as well that were previously inherited from |
|
It's certainly breaking some tests. Do you perhaps want to take it from here? I don't feel strongly enough about this change to spend a lot of time on it. |
I'll have a closer look. |
|
It's also perfectly fine to re-add on |
I think it's breaking almost all the tests. |
ebdb6a8
to
550063d
Compare
|
I've been looking at this a bit. I think |
Sounds fine. In that case it might as well extend |
This fixes all the tests (see the internal PR that now links to this one). Given the nature of this change, I'm tempted to make this one breaking instead of having a deprecation period. However, I'll need to check with some people that this reasonable. |
Follow up to #14321
With the above-mentioned PR,
Containernow has conflicting urls provided bygetURLandgetLocation, respectively. This PR removesgetLocation, sincegetURLought to be sufficient. I was a bit in doubt about whether to switch theextendstoElementorElementBase, but it seemed that the additional predicates inherited fromElementmade little sense forContainers, so I choseElementBase.cc @jketema