-
Notifications
You must be signed in to change notification settings - Fork 866
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
Fixing rename for permitted subclasses. #7977
base: master
Are you sure you want to change the base?
Conversation
(Found a problem in the patch.) |
a77b372
to
d14199a
Compare
Should be fixed now, and ready for review. Please let me know what you think! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks good as far as I can tell (thanks for the comments in CasualDiff).
merge conflict in apichanges.xml needs to be resolved + rebasing would be good to have a fresh CI run before integration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a regression right after review :(
if you rename the interface, it will remove the permits section, this does not happen in NB 24
FWIW: the pre-existing methods now are a bit problematic, as if some code takes a So, I've tried to go through the whole NetBeans (production code), and fix what I could. Will push that shortly. I think I still need to make at least one pass through it, maybe add some tests, and maybe add some automated way to prevent use of the pre-existing methods. Will try to look at that as I can. |
Having something like:
Renaming
Subtype
will not rename the identifier in thepermits
clause. This is because:a) the write model (
CausalDiff
) does not supportpermits
b) the indexing does not include the permitted subclasses.
This PR is trying to fix both.