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.

Sign up for GitHub

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

Jump to bottom

Don't parse Version#num for reads #610

Merged
merged 1 commit into from
Mar 11, 2017
Merged

Don't parse Version#num for reads #610

merged 1 commit into from
Mar 11, 2017

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Mar 10, 2017

On the majority of endpoints involving versions, we're parsing the
version (using .unwrap even, which could take down the server) only to
immediately convert it back into a string. This causes needless
allocations, and a larger surface for errors to appear.

We really only care about semver when ensuring a version is valid, and
when determining the max version for a crate. Everywhere else, we can
just use a String.

@sgrif
Don't parse Version#num for reads
97e55d3 
On the majority of endpoints involving versions, we're parsing the
version (using `.unwrap` even, which could take down the server) only to
immediately convert it back into a string. This causes needless
allocations, and a larger surface for errors to appear.

We really only care about semver when ensuring a version is valid, and
when determining the max version for a crate. Everywhere else, we can
just use a String.
@sgrif sgrif force-pushed the sg-dont-parse-semver-for-reads branch from 91c4451 to 97e55d3 Compare March 10, 2017 14:28
@carols10cents
Copy link
Member

(using .unwrap even, which could take down the server)

wellllll only if the database gets something in it that doesn't parse

carols10cents
carols10cents reviewed Mar 11, 2017
View reviewed changes
src/krate.rs
@@ -64,7 +65,7 @@ pub struct EncodableCrate {
pub badges: Option<Vec<EncodableBadge>>,
pub created_at: String,
pub downloads: i32,
pub max_version: String,
pub max_version: Cow<'static, str>,
Copy link
Member

Choose a reason for hiding this comment

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

🐮

@carols10cents
Copy link
Member

lgtm!

@carols10cents carols10cents merged commit a64ca12 into rust-lang:master Mar 11, 2017
@sgrif sgrif deleted the sg-dont-parse-semver-for-reads branch March 11, 2017 01:14
@sgrif sgrif mentioned this pull request Mar 11, 2017
Merged
bors added a commit that referenced this pull request Dec 18, 2019
@bors
Auto merge of #1973 - rust-lang:dependabot/npm_and_yarn/eslint-plugin…
c1cbe41 
…-ember-7.7.2, r=Turbo87

Bump eslint-plugin-ember from 7.0.0 to 7.7.2

Bumps [eslint-plugin-ember](https://github.com/ember-cli/eslint-plugin-ember) from 7.0.0 to 7.7.2.
<details>
<summary>Release notes</summary>

*Sourced from [eslint-plugin-ember's releases](https://github.com/ember-cli/eslint-plugin-ember/releases).*

> ## v7.7.2
> #### 🐛 Bug Fix
> * [#621](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/621) Fix false positive with `ignoreNonThisExpressions` option in `use-ember-get-and-set` rule ([@&#8203;Exelord](https://github.com/Exelord))
>
> #### 📝 Documentation
> * [#620](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/620) Use consistent prefixes for rule descriptions ([@&#8203;bmish](https://github.com/bmish))
>
> #### 🏠 Internal
> * [#625](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/625) Add eslint-plugin-jest internally and enable rules ([@&#8203;bmish](https://github.com/bmish))
> * [#624](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/624) Add eslint-plugin-unicorn internally and enable recommended rules ([@&#8203;bmish](https://github.com/bmish))
>
> #### Committers: 2
> - Bryan Mishkin ([@&#8203;bmish](https://github.com/bmish))
> - Maciej Kwaśniak ([@&#8203;Exelord](https://github.com/Exelord))
>
> ## v7.7.1
> #### 🐛 Bug Fix
> * [#615](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/615) Fix issue causing assert to fire in `getSourceModuleName` util function ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> #### Committers: 1
> - Pat O'Callaghan ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> ## v7.7.0
> #### 🚀 Enhancement
> * [#592](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/592) Update `no-classic-classes` rule to catch classic Ember Data model classes ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> #### 🐛 Bug Fix
> * [#610](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/610) Fix invalid `no-get` rule autofix caused by invalid JS variable name ([@&#8203;bmish](https://github.com/bmish))
> * [#607](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/607) Fix spread property bug in `require-super-in-init` rule ([@&#8203;bmish](https://github.com/bmish))
> * [#600](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/600) Add missing schema validation for options on many rules ([@&#8203;bmish](https://github.com/bmish))
>
> #### 🏠 Internal
> * [#611](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/611) Add many missing tests for lines without test coverage ([@&#8203;bmish](https://github.com/bmish))
>
> #### Committers: 2
> - Bryan Mishkin ([@&#8203;bmish](https://github.com/bmish))
> - Pat O'Callaghan ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> ## v7.6.0
> #### 🚀 Enhancement
> * [#594](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/594) Add new rule `no-get-with-default` ([@&#8203;steventsao](https://github.com/steventsao))
>
> #### Committers: 1
> - Steven Tsao ([@&#8203;steventsao](https://github.com/steventsao))
>
> ## v7.5.0
> #### 🚀 Enhancement
> * [#583](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/583) Update `no-observers` rule to handle decorators ([@&#8203;bmish](https://github.com/bmish))
> * [#577](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/577) Add autofixer to `no-get` rule ([@&#8203;bmish](https://github.com/bmish))
>
></tr></table> ... (truncated)
</details>
<details>
<summary>Changelog</summary>

*Sourced from [eslint-plugin-ember's changelog](https://github.com/ember-cli/eslint-plugin-ember/blob/master/CHANGELOG.md).*

> ## v7.7.2 (2019-12-12)
>
> #### 🐛 Bug Fix
> * [#621](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/621) Fix false positive with `ignoreNonThisExpressions` option in `use-ember-get-and-set` rule ([@&#8203;Exelord](https://github.com/Exelord))
>
> #### 📝 Documentation
> * [#620](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/620) Use consistent prefixes for rule descriptions ([@&#8203;bmish](https://github.com/bmish))
>
> #### 🏠 Internal
> * [#625](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/625) Add eslint-plugin-jest internally and enable rules ([@&#8203;bmish](https://github.com/bmish))
> * [#624](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/624) Add eslint-plugin-unicorn internally and enable recommended rules ([@&#8203;bmish](https://github.com/bmish))
>
> #### Committers: 2
> - Bryan Mishkin ([@&#8203;bmish](https://github.com/bmish))
> - Maciej Kwaśniak ([@&#8203;Exelord](https://github.com/Exelord))
>
> ## v7.7.1 (2019-11-29)
>
> #### 🐛 Bug Fix
> * [#615](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/615) Fix issue causing assert to fire in `getSourceModuleName` util function ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> #### Committers: 1
> - Pat O'Callaghan ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> ## v7.7.0 (2019-11-29)
>
> #### 🚀 Enhancement
> * [#592](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/592) Update `no-classic-classes` rule to catch classic Ember Data model classes ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> #### 🐛 Bug Fix
> * [#610](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/610) Fix invalid `no-get` rule autofix caused by invalid JS variable name ([@&#8203;bmish](https://github.com/bmish))
> * [#607](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/607) Fix spread property bug in `require-super-in-init` rule ([@&#8203;bmish](https://github.com/bmish))
> * [#600](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/600) Add missing schema validation for options on many rules ([@&#8203;bmish](https://github.com/bmish))
>
> #### 🏠 Internal
> * [#611](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/611) Add many missing tests for lines without test coverage ([@&#8203;bmish](https://github.com/bmish))
>
> #### Committers: 2
> - Bryan Mishkin ([@&#8203;bmish](https://github.com/bmish))
> - Pat O'Callaghan ([@&#8203;patocallaghan](https://github.com/patocallaghan))
>
> ## v7.6.0 (2019-11-19)
>
> #### 🚀 Enhancement
> * [#594](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/pull/594) Add new rule `no-get-with-default` ([@&#8203;steventsao](https://github.com/steventsao))
>
> #### Committers: 1
> - Steven Tsao ([@&#8203;steventsao](https://github.com/steventsao))
>
> ## v7.5.0 (2019-11-11)
></tr></table> ... (truncated)
</details>
<details>
<summary>Commits</summary>

- [`1805883`](ember-cli/eslint-plugin-ember@1805883) v7.7.2
- [`39539bc`](ember-cli/eslint-plugin-ember@39539bc) Update CHANGELOG
- [`9943b7e`](ember-cli/eslint-plugin-ember@9943b7e) Merge pull request [#625](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/issues/625) from bmish/eslint-plugin-jest
- [`cb4824d`](ember-cli/eslint-plugin-ember@cb4824d) chore(lint): add eslint-plugin-jest internally and enable rules
- [`dc149b5`](ember-cli/eslint-plugin-ember@dc149b5) Merge pull request [#624](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/issues/624) from bmish/eslint-plugin-unicorn
- [`9a6a4be`](ember-cli/eslint-plugin-ember@9a6a4be) chore(lint): add eslint-plugin-unicorn internally and autofix recommended rules
- [`c93198c`](ember-cli/eslint-plugin-ember@c93198c) build(deps-dev): bump eslint-plugin-import from 2.18.2 to 2.19.1 ([#623](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/issues/623))
- [`e2c5235`](ember-cli/eslint-plugin-ember@e2c5235) build(deps): bump snake-case from 3.0.1 to 3.0.2 ([#622](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/issues/622))
- [`4c6aedd`](ember-cli/eslint-plugin-ember@4c6aedd) Merge pull request [#621](https://github-redirect.dependabot.com/ember-cli/eslint-plugin-ember/issues/621) from Exelord/fix/ignore-this-expressions
- [`ed6301a`](ember-cli/eslint-plugin-ember@ed6301a) chore: fix lint violation
- Additional commits viewable in [compare view](ember-cli/eslint-plugin-ember@v7.0.0...v7.7.2)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=eslint-plugin-ember&package-manager=npm_and_yarn&previous-version=7.0.0&new-version=7.7.2)](https://dependabot.com/compatibility-score.html?dependency-name=eslint-plugin-ember&package-manager=npm_and_yarn&previous-version=7.0.0&new-version=7.7.2)

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carols10cents carols10cents carols10cents left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sgrif @carols10cents