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

V0.2.0 #12

Merged
merged 5 commits into from
Jul 25, 2019
Merged

V0.2.0 #12

merged 5 commits into from
Jul 25, 2019

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Jul 25, 2019

No description provided.

- use new syntax for license
- use default exclude rules
@matklad
Copy link
Contributor Author

matklad commented Jul 25, 2019

wait a sec, want to pull other minor changes here

@matklad matklad mentioned this pull request Jul 25, 2019
@lnicola
Copy link

lnicola commented Jul 25, 2019

Should this be 0.2.0?

@Manishearth
Copy link
Member

Yeah, make this 2.0

@matklad
Copy link
Contributor Author

matklad commented Jul 25, 2019

@Manishearth hm, than let's make 1.0.0 then?

@Manishearth
Copy link
Member

I'd rather not for now.

@Manishearth
Copy link
Member

I'm okay with a 1.0.0, I just don't have the bandwidth to do the reviews for it.

@matklad
Copy link
Contributor Author

matklad commented Jul 25, 2019

Actually, let me write this down in the long form:

I think upgrading unicode version for this crate should be considered a minor (that is, non-semver breaking) change. This is nice, because all depndencies will support newer unicode versions "for free".

So, it makes sense to release this as 0.1.1 (remember, if major is 0, then patch means minor).

OTOH, we really should do an 1.0.0 for this crate, and this is a good occasion for this! 1.0.0 shouldn't be disrupting, because this is not an interface crate. Making 1.0.0 makes sense, because literally half of the ecosystem depends on this crate via proc_macro2.

If we want to treat unicode upgrades as semver breaking, than yes, it makes sense to do 0.2.0.

However, I don't have a strong opinion here, so I'll just optimistically change this to 0.2.0. I'll also be happy to help with maintenance :)

@Manishearth
Copy link
Member

Travis fails on some tests.

Yeah, I think in general we've made unicode updates breaking changes. But perhaps we shouldn't once we have 1.0.0.

Note that unicode updates often come with algorithm changes (this one doesn't).

Totally down for a 1.0.0, someone else needs to do the reviewing work.

@matklad matklad changed the title V0.1.1 V0.2.0 Jul 25, 2019
@Manishearth
Copy link
Member

For some reason this implementation is 10x as slow as the stdlib, btw

@Manishearth Manishearth reopened this Jul 25, 2019
@Manishearth Manishearth merged commit 4baae9f into unicode-rs:master Jul 25, 2019
@matklad
Copy link
Contributor Author

matklad commented Jul 25, 2019

@Manishearth they use different algorithms: this crate uses binary search, stdlib uses a trie.

Totally down for a 1.0.0, someone else needs to do the reviewing work.

I think if we decide to move compiler over to this crate instead of libcore, t-compiler will be in charge of maintaining this (and will probably do 1.0)

@Manishearth
Copy link
Member

Sounds good.

@Manishearth
Copy link
Member

Should we also be using a trie?

@matklad
Copy link
Contributor Author

matklad commented Jul 25, 2019

That's code-size/time trade off iiuc. I think binary search is a better default, because ASCII typically has a different fast-path, so we are not that concerned with performance.

The "proper" solution here is to do both, via a feature flag. Should be relatively easy to do if we switch from python to https://github.com/BurntSushi/ucd-generate

@matklad matklad deleted the v0.1.1 branch July 25, 2019 13:31
This was referenced Jul 25, 2019
@lnicola
Copy link

lnicola commented Jul 27, 2019

Perhaps a fast-path for ASCII characters would recover any performance loss here.

because ASCII typically has a different fast-path

Ah...

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

Successfully merging this pull request may close these issues.

3 participants