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

Remove DirectIndexString from Base #24259

Closed
bkamins opened this issue Oct 21, 2017 · 3 comments
Closed

Remove DirectIndexString from Base #24259

bkamins opened this issue Oct 21, 2017 · 3 comments
Labels
domain:strings "Strings!" kind:excision Removal of code from Base or the repository

Comments

@bkamins
Copy link
Member

bkamins commented Oct 21, 2017

It was discussed in #23805 that DirectIndexString should be moved from Base to LegacyStrings.

I can start working on it. However, I am not sure about the expected details of this process. In particular:

  • should it be moved to LegacyStrings first and after it is merged there be removed from Base (those are two separate repositories);
  • nextind and prevind for DirectIndexString are inconsistent with other AbstractString types - should this be fixed or left as is for backward compatibility?
  • I understand that this code should be included only under versions later than 0.6 - is the best practice for that to wrap the code in if VERSION >= v"0.6.0" block?

CC @nalimilan @ararslan @StefanKarpinski

@ararslan
Copy link
Member

I recommend taking a look at the structure of LegacyStrings. As an example, I have a PR up there that adds RevString. Putting stuff in there is pretty straightforward. You just move the type and all of the Base methods for it.

@ararslan ararslan added kind:excision Removal of code from Base or the repository domain:strings "Strings!" status:triage This should be discussed on a triage call labels Oct 21, 2017
@nalimilan
Copy link
Member

No need to change the behavior of prevind and nextind since DirectIndexString will be semi-deprecated in LegacyStrings anyway.

@bkamins
Copy link
Member Author

bkamins commented Oct 22, 2017

close?

@JeffBezanson JeffBezanson removed the status:triage This should be discussed on a triage call label Oct 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:strings "Strings!" kind:excision Removal of code from Base or the repository
Projects
None yet
Development

No branches or pull requests

5 participants