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

📦 NEW: Add an option to sort data (#3) via @smitp #3

Merged
merged 2 commits into from Mar 20, 2020

Conversation

@smitp
Copy link
Contributor

smitp commented Mar 20, 2020

...

@smitp smitp force-pushed the smitp:master branch from f9d5a1e to 420814f Mar 20, 2020
Copy link
Owner

ahmadawais left a comment

Thanks for this. I was hoping to work on it later. Couple of questions and changes. Check comments.

},
sort: {
type: 'string',
default: '',

This comment has been minimized.

Copy link
@ahmadawais

ahmadawais Mar 20, 2020

Owner

Should be false?

`per-million`
],
sortOrders: [
1,

This comment has been minimized.

Copy link
@ahmadawais

ahmadawais Mar 20, 2020

Owner

I like the sort by death count that's set by default. Gives you the best picture. What do you say we make that default instead of sorting by countries?

This comment has been minimized.

Copy link
@smitp

smitp Mar 20, 2020

Author Contributor

It only sorts if sort option is provided. sortOrders are to specify asc/desc

I like the idea of sorting by cases today. That gives a better picture of situation than death count.

Copy link
Owner

ahmadawais left a comment

Check the comments.

@smitp smitp requested a review from ahmadawais Mar 20, 2020
Copy link
Owner

ahmadawais left a comment

Looks good to me! ✔︎

@ahmadawais ahmadawais changed the title Add an option to sort data 📦 NEW: Add an option to sort data (#3) via @smitp Mar 20, 2020
@ahmadawais ahmadawais merged commit 3e27c86 into ahmadawais:master Mar 20, 2020
@ahmadawais

This comment has been minimized.

Copy link
Owner

ahmadawais commented Mar 20, 2020

Thank you, @smitp — I'm adding a couple more new features and releasing the new CLI in a few.

@ahmadawais

This comment has been minimized.

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.