Normalise keywords to lowercase when searching for crates. #140

huonw · 2015-03-20T07:11:24Z

Currently keyword search is case-sensitive, but this is often unhelpful,
as they are heuristic classification tools, not hard guarantees. It's
not particularly helpful to search for
"fft" and not get the crates tagged
"FFT".

Closes #100.

huonw · 2015-03-20T07:11:39Z

I don't have a testing set-up set up, unfortunately.

alexcrichton · 2015-03-20T17:42:46Z

src/bin/migrate.rs

@@ -418,6 +418,11 @@ fn migrations() -> Vec<Migration> {
                             crate_owners_unique_user_per_crate", &[]));
            Ok(())
        }),
+        Migration::new(20150320174400, |tx| {
+            try!(tx.execute("ALTER TABLE keyword SET keyword = lower(keyword)",
+                            &[]));


I think that this should be ALTER TABLE keywords (although I also thought it would be UPDATE keywords ...?).

I think that this may also not land well in production as there's currently a uniqueness constraint on the keyword column, but this will generate duplicates (e.g. FFT => fft).

I think that this may want to iterate through all keywords, deleting those with a downcase equivalent and migrating all current members of that keyword to the downcase equivalent. This should also probably delete the old uniqueness constraint and add a new one on lower(keyword) to speed up queries and provide a db-level gurantee that we don't have duplicates.

Oh, I totally mangled this.

alexcrichton · 2015-03-20T17:44:25Z

Nice! Could you also add a test where two crates are uploaded with two different cases of a keyword, but they're canonicalized to just one?

For crates at least we also search case insensitively but insert with the case provided. In that case we may want to preserve the case of the keyword given to us when we insert into the database.

huonw · 2015-03-22T00:53:04Z

Could you also add a test where two crates are uploaded with two different cases of a keyword, but they're canonicalized to just one?

I'm not sure I understand what this would look like.

huonw · 2015-03-22T02:13:10Z

For crates at least we also search case insensitively but insert with the case provided. In that case we may want to preserve the case of the keyword given to us when we insert into the database.

Updated with this approach.

alexcrichton · 2015-03-22T17:54:03Z

Could you also add a test where two crates are uploaded with two different cases of a keyword, but they're canonicalized to just one?

I'm not sure I understand what this would look like.

Oh nevermind, I thought that this was going to prevent creation of keywords with the same name that differed only in case, but I think I prefer this approach instead.

Thanks!

alexcrichton · 2015-03-22T17:54:28Z

src/bin/migrate.rs

@@ -440,6 +440,14 @@ fn migrations() -> Vec<Migration> {
                             ON crates (lower(name))", &[]));
            Ok(())
        }),
+        Migration::new(20150320174400, |tx| {
+            try!(tx.execute("CREATE INDEX index_keywords_lower_keyword ON (lower(keyword))",


I think this is missing the table name

This file also seems to have made itself executable?

I'm not doing a very good job of writing correct code... :/

Fixed both.

Currently keyword search is case-sensitive, but this is often unhelpful, as they are heuristic classification tools, not hard guarantees. It's not particularly helpful to search for ["fft"](https://crates.io/keywords/fft) and not get the crates tagged ["FFT"](https://crates.io/keywords/FFT). Closes rust-lang#100.

alexcrichton · 2015-03-23T16:49:40Z

Looks good to me! I'll merge once it agrees with the nightlies again.

alexcrichton · 2015-03-23T18:18:10Z

Merged as 4658cf3, botched the merge but oh well :(

….6.0, r=locks Bump ember-percy from 1.4.0 to 1.6.0 Bumps [ember-percy](https://github.com/percy/ember-percy) from 1.4.0 to 1.6.0. <details> <summary>Release notes</summary> *Sourced from [ember-percy's releases](https://github.com/percy/ember-percy/releases).* > ## v1.6.0 > ## What changed? > > This version of ember-percy includes more from input serialization when taking the DOM snapshot. You should now see all form inputs properly capture their state ([#146](https://github-redirect.dependabot.com/percy/ember-percy/issues/146)) > > ## v1.5.0 > Bower was removed - [#73](https://github-redirect.dependabot.com/percy/ember-percy/issues/73) 🎉 🌮 🎉 🗡 > Node 4 was removed from testing - [#75](https://github-redirect.dependabot.com/percy/ember-percy/issues/75) & [#74](https://github-redirect.dependabot.com/percy/ember-percy/issues/74) > PERCY_PROJECT environment variable is no longer required - [#72](https://github-redirect.dependabot.com/percy/ember-percy/issues/72) 🚀 > Some defensive programming was added - [#71](https://github-redirect.dependabot.com/percy/ember-percy/issues/71) (thanks [@Gaurav0](https://github.com/Gaurav0)!) 🛡 > > ## v1.4.4 > **Important bugfix, please upgrade:** > - Fix duplicate asset clobbering resource paths. ([#70](https://github-redirect.dependabot.com/percy/ember-percy/issues/70)) > > ## v1.4.3 > Do not release yarn.lock to npm. Thanks [@Turbo87](https://github.com/Turbo87)! > > ## v1.4.2 > Fix jQuery version conflict bug. ([#65](https://github-redirect.dependabot.com/percy/ember-percy/issues/65)) > > ## v1.4.1 > 🐛 Reintroduce support for older ember apps that don't know about `@ember/test` yet. </details> <details> <summary>Commits</summary> - [`dd7288c`](percy/percy-ember@dd7288c) v1.6.0 - [`a31d5b4`](percy/percy-ember@a31d5b4) feat: Add more form element serialization ([#146](https://github-redirect.dependabot.com/percy/ember-percy/issues/146)) - [`e387464`](percy/percy-ember@e387464) build(deps): Bump handlebars from 4.0.11 to 4.2.0 ([#145](https://github-redirect.dependabot.com/percy/ember-percy/issues/145)) - [`868783a`](percy/percy-ember@868783a) build(deps): Bump underscore.string from 3.3.4 to 3.3.5 ([#144](https://github-redirect.dependabot.com/percy/ember-percy/issues/144)) - [`8442380`](percy/percy-ember@8442380) build(deps-dev): Bump ember-cli-htmlbars-inline-precompile ([#140](https://github-redirect.dependabot.com/percy/ember-percy/issues/140)) - [`9f503a7`](percy/percy-ember@9f503a7) build(deps-dev): Bump eslint-plugin-node from 9.1.0 to 9.2.0 ([#143](https://github-redirect.dependabot.com/percy/ember-percy/issues/143)) - [`d0786ac`](percy/percy-ember@d0786ac) build(deps): Bump percy-client from 3.0.12 to 3.0.13 ([#142](https://github-redirect.dependabot.com/percy/ember-percy/issues/142)) - [`29877a5`](percy/percy-ember@29877a5) build(deps): Bump ember-cli-babel from 7.10.0 to 7.11.0 ([#141](https://github-redirect.dependabot.com/percy/ember-percy/issues/141)) - [`0109d54`](percy/percy-ember@0109d54) build(deps-dev): Bump ember-load-initializers from 2.0.0 to 2.1.0 ([#139](https://github-redirect.dependabot.com/percy/ember-percy/issues/139)) - [`e8c6f35`](percy/percy-ember@e8c6f35) build(deps): [Security] Bump mixin-deep from 1.3.1 to 1.3.2 ([#138](https://github-redirect.dependabot.com/percy/ember-percy/issues/138)) - Additional commits viewable in [compare view](percy/percy-ember@v1.4.0...v1.6.0) </details> <details> <summary>Maintainer changes</summary> This version was pushed to npm by [percy-admin](https://www.npmjs.com/~percy-admin), a new releaser for ember-percy since your current version. </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=ember-percy&package-manager=npm_and_yarn&previous-version=1.4.0&new-version=1.6.0)](https://dependabot.com/compatibility-score.html?dependency-name=ember-percy&package-manager=npm_and_yarn&previous-version=1.4.0&new-version=1.6.0) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- **Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit. You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com). <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com): - Update frequency (including time of day and day of week) - Pull request limits (per update run and/or open at any time) - Automerge options (never/patch/minor, and dev/runtime dependencies) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) </details>

alexcrichton reviewed Mar 20, 2015
View reviewed changes

huonw force-pushed the low-key branch from 23d0863 to 56aa080 Compare March 22, 2015 01:12

huonw changed the title ~~Normalise keywords to lowercase.~~ Normalise keywords to lowercase when searching for crates. Mar 22, 2015

huonw force-pushed the low-key branch 2 times, most recently from 5af73bd to 6af7d25 Compare March 22, 2015 01:35

alexcrichton reviewed Mar 22, 2015
View reviewed changes

huonw force-pushed the low-key branch from 6af7d25 to 80dc1d2 Compare March 23, 2015 01:04

alexcrichton closed this Mar 23, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normalise keywords to lowercase when searching for crates. #140

Normalise keywords to lowercase when searching for crates. #140

huonw commented Mar 20, 2015

huonw commented Mar 20, 2015

alexcrichton Mar 20, 2015

huonw Mar 22, 2015

alexcrichton commented Mar 20, 2015

huonw commented Mar 22, 2015

huonw commented Mar 22, 2015

alexcrichton commented Mar 22, 2015

alexcrichton Mar 22, 2015

alexcrichton Mar 22, 2015

huonw Mar 23, 2015

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015

Normalise keywords to lowercase when searching for crates. #140

Normalise keywords to lowercase when searching for crates. #140

Conversation

huonw commented Mar 20, 2015

huonw commented Mar 20, 2015

alexcrichton Mar 20, 2015

Choose a reason for hiding this comment

huonw Mar 22, 2015

Choose a reason for hiding this comment

alexcrichton commented Mar 20, 2015

huonw commented Mar 22, 2015

huonw commented Mar 22, 2015

alexcrichton commented Mar 22, 2015

alexcrichton Mar 22, 2015

Choose a reason for hiding this comment

alexcrichton Mar 22, 2015

Choose a reason for hiding this comment

huonw Mar 23, 2015

Choose a reason for hiding this comment

alexcrichton commented Mar 23, 2015

alexcrichton commented Mar 23, 2015