Skip to content
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

Rename ord_at to unicode_at for String #15522

Closed
wants to merge 1 commit into from
Closed

Rename ord_at to unicode_at for String #15522

wants to merge 1 commit into from

Conversation

Geequlim
Copy link
Contributor

@Geequlim Geequlim commented Jan 9, 2018

Close #15519

</argument>
<description>
Returns the character code at position [code]at[/code].
Returns the character code at position [code]idx[/code].
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You changed the tab to spaces.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry clang-formate doesn't work for me on Windows. I will fix it when I get home

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format doesn't apply to the XML, for those you need to preserve the formatting manually.

@akien-mga
Copy link
Member

GDNative fails building it seems:

modules/gdnative/gdnative_api_struct.gen.cpp:706:2: error: 'godot_string_ord_at' was not declared in this scope
  godot_string_ord_at,
  ^

@akien-mga
Copy link
Member

I don't think it's worth breaking the API this late in the release cycle, and we only accept fixes to critical issues now (+ some enhancements to recent features like mono, bullet, gdnative, autotile, etc.).

We can consider it for 3.1 / 3.0.x, but then it will have to be added in a way that does not break compatibility (though ord_at should be kept, albeit with a deprecation warning announcing when it will be removed and pointing to the new method).

@akien-mga akien-mga added this to the 3.1 milestone Jan 9, 2018
@Geequlim
Copy link
Contributor Author

@akien-mga Is there a way to flag a method is deprecated?

@akien-mga
Copy link
Member

Not yet, but there will be when we implement #4397.

@akien-mga
Copy link
Member

Closing for now as we need a proper deprecation workflow before this can be implemented. I've listed it in #16863, so we can rediscuss it and maybe make the change later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants