-
Notifications
You must be signed in to change notification settings - Fork 591
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
Remove Support for the tag
Keyword
#2177
Remove Support for the tag
Keyword
#2177
Conversation
tag
keyword.tag
Keyword
I thought our idea was to use the word tag going forward to limit confusion with "optional" and that tag would be prefered. Pretty sure we're using "tag" in various Input/Output streams and in some Slice. Is there an issue/discussion for removing this? |
That's going to be a tall order. Updating all our code/docs to use
We're only using
No, I can make a proper issue if that's preferable to discussing it here on this PR. |
I'm fine to remove it I guess but I'm pretty sure we've been sprinkling the word tag all around the code since we started on 3.8 and added tag as an alias for optional. https://github.com/zeroc-ice/ice/blob/main/cpp/include/Ice/StreamHelpers.h#L396 I'm sure there are more. This commit introduced several that should be undone. 885ad85 |
Yes, you're right. I'll look around the code for any references to |
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.
Sounds good
e5494fd
to
4aedb2f
Compare
We had a separate issue open for removing the 'tagged's that snuck in: #1623 |
I altered this PR to also fix any mentions of the |
In the "Ice 4.0" days, we replaced the
optional
keyword totag
, but keptoptional
for backwards compatibility.We kept separate tokens for them so we could emit deprecation warnings for
optional
. Different behavior == different tokens.Now that "Ice 3.8" is the target, I see no point in supporting
tag
, so this PR deletes it.This keyword doesn't exist in
3.7
, and there's no reason for anyone to switch fromoptional
totag
.There isn't much to review just wanted to make sure everyone was aware of & okay with this.
Note that we already removed the deprecation warning. Or maybe it never got backported in the first place.
Without it there is no difference in behavior between the tokens. The code is literally copy/paste duplicates.