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

added options to the module - sort, order #40

Closed
wants to merge 3 commits into from
Closed

added options to the module - sort, order #40

wants to merge 3 commits into from

Conversation

maddhruv
Copy link
Member

No description provided.

@maddhruv maddhruv requested a review from bnb November 18, 2018 13:28
@bnb
Copy link
Member

bnb commented Nov 20, 2018

If we are doing this in the module, we should probably also remove the sort from each of the queries in data/projects.json.

@bnb bnb added this to the 0.14.0 milestone Nov 20, 2018
@maddhruv
Copy link
Member Author

as this.sort == query.sort
we can remove the query.sort 👍

@bnb
Copy link
Member

bnb commented Nov 21, 2018

If you could do that in this PR we can land it 👍

@bnb bnb modified the milestones: 0.14.0, 0.15.0 Nov 21, 2018
@maddhruv
Copy link
Member Author

did it

@bnb
Copy link
Member

bnb commented Nov 22, 2018

A few notes on this:

  • We're passing the options directly to Octokit, which is AWESOME! This means that per_page and page work by default.
    • We should also document that per_page and page work just fine when passed 👍
  • We're also enabling users to pass q, which overwrites the Good First issue query defined by the passed project.
    • Instead of putting q into a black hole so it won't overwrite our queries, we should try to leverage this ability to dynamically add additional context to the base query

Here's an image of what happens when I pass q: 'gcc' to a custom example I built out to test this (I'll submit it as a PR separately):

image

We need to handle q before landing this 👍

@bnb bnb modified the milestones: 0.15.0, 0.16.0 Nov 25, 2018
@bnb
Copy link
Member

bnb commented Nov 25, 2018

Since we've got some open PRs that can currently land and were waiting on this one as 0.15.0, I've changed this to 0.16.0 and am going to ship another as 0.15.0 for now 👍

This was referenced Nov 25, 2018
@bnb bnb removed this from the 0.16.0 milestone Nov 29, 2018
@bnb
Copy link
Member

bnb commented Nov 29, 2018

Going to remove the milestone for now until this is ready to land, at which point it will be whatever the next minor is.

@bnb bnb added the enhancement New feature or request label Dec 16, 2018
@bnb
Copy link
Member

bnb commented Jul 25, 2019

@maddhruv is this still a goal? If so, can it be cleaned up? If not, can it be closed? 🙏

@maddhruv
Copy link
Member Author

as of now
it is not
and has gone a little messy
closing

@maddhruv maddhruv closed this Jul 26, 2019
@maddhruv maddhruv deleted the module branch July 26, 2019 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants