Skip to content

Conversation

@carols10cents
Copy link
Member

I haven't figured out a great way to add a test for this because I need to create a crate and get a crate ID and then change the app's config, and the test app doesn't easily have this... so I thought I'd throw up this change for discussion and see if anyone had better ideas.

@Turbo87
Copy link
Member

Turbo87 commented Mar 18, 2022

would it be possible to filter by crate name instead? or is the name not available in that table?

@carols10cents
Copy link
Member Author

The query does join to the crates table, so the crate name is available

@Turbo87
Copy link
Member

Turbo87 commented Mar 18, 2022

in that case I think you can start a test app with a random crate name in the new config value and then create that crate after the app has started, right?

@Turbo87 Turbo87 added C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear A-backend ⚙️ labels Mar 18, 2022
@Turbo87
Copy link
Member

Turbo87 commented Mar 18, 2022

given that the particular crate now has almost 1/4 of the downloads of bitflags in total, do we also want to exclude it from the "Most downloaded" list?

@carols10cents
Copy link
Member Author

Good calls. Updated!

@bors
Copy link
Contributor

bors commented Mar 18, 2022

☔ The latest upstream changes (presumably #4645) made this pull request unmergeable. Please resolve the merge conflicts.

@Turbo87
Copy link
Member

Turbo87 commented Mar 20, 2022

looks great, thank you!

one potential follow up: we allow the users to view the list of crates sorted by downloads too, but we don't exclude the crates there. I'm not sure how or if we should deal with that too.

merging anyways since this is a good first step :)

@bors r+

@bors
Copy link
Contributor

bors commented Mar 20, 2022

📌 Commit bf912ec has been approved by Turbo87

@bors
Copy link
Contributor

bors commented Mar 20, 2022

⌛ Testing commit bf912ec with merge 1bcbfb1...

@bors
Copy link
Contributor

bors commented Mar 20, 2022

☀️ Test successful - checks-actions
Approved by: Turbo87
Pushing 1bcbfb1 to master...

@bors bors merged commit 1bcbfb1 into master Mar 20, 2022
@bors bors deleted the exclude-top branch March 20, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-backend ⚙️ C-internal 🔧 Category: Nonessential work that would make the codebase more consistent or clear

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants