-
Notifications
You must be signed in to change notification settings - Fork 14
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
Deleted attributes aren't remove completely from XML #373
Comments
When working on #750 where I included special handling for inserted attributes, I took a look at how deletion works. For reference, I will include that here, too. How it worksA deletion is represented as follows:
recheck assumes that When applying an (identifying) attribute difference, it is passed down to the respective attributes collection ( The ProposalOne could assume that we simply remove that attribute from the collection. However, this is not the case, since some attributes may require special handling (e.g. the path attribute should never be removed, see IdentifyingAttributesTest). With that in mind, I would propose either of the following:
The first approach is based on the assumption that special handling is required (currently only path) and might not even be the case anymore. I think, that this special handling is the responsibility of the extension at hand, not of the user applying the change. Thus these cases should be handled earlier and should not be reported to the user on apply. The second approach is more clean, as the collection can decide when to remove a attribute., thus I prefer this approach. For the special handling is the decision to be made, if it should be re-implemented or simply removed for now. Thus an extension has to respect and enforce this. |
Consider the following change:
Adapted HTML (removes
id
attribute completely): https://github.com/retest/recheck-web/pull/297/files#diff-b980e03c12cee358e1072d97aedd8dedR165Our GM after apply via review: https://github.com/retest/recheck-web/pull/297/files#diff-b980e03c12cee358e1072d97aedd8dedR165
As can be seen, the XML element for the
id
attribute is now empty, but it is still part of the XML—although it doesn't exist anymore.We should ensure that stuff which has been deleted gets also deleted from our GMs, especially for space reasons. While doing so, we should also ensure that this is the default in similar situations, e.g., when an element, identifying attribute, etc. gets deleted.
The text was updated successfully, but these errors were encountered: