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

feat: [sveltekit pod] missing issues filters #979

Conversation

ihardz
Copy link
Contributor

@ihardz ihardz commented Nov 28, 2022

Partially resolves #901

Includes:

  • milestone service
  • milestone filter
  • dropdown adjustments (appearance direction)

@ihardz ihardz self-assigned this Nov 28, 2022
@ihardz ihardz changed the title 901 sveltekit pod issues missing filters feat: [sveltekit pod] issues missing filters Nov 28, 2022
@ihardz ihardz changed the title feat: [sveltekit pod] issues missing filters feat: [sveltekit pod] missing issues filters Nov 28, 2022
@ihardz ihardz linked an issue Nov 29, 2022 that may be closed by this pull request
Copy link
Contributor

@BigAB BigAB left a comment

Choose a reason for hiding this comment

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

Some small changes requested and bigger chanegs noted but that can wait

const body = await response.json();
throw error(response.status, body?.message || response.statusText);
}
const responseBodyJson = (await response.json()) as Promise<GithubIssueMilestone[]>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally I think casting the type with as is a code smell. Sometimes it can't be avoided, but it shouldn't be the goto solution.

I would rather see something just validate the api response is of GithubIssueMilestone type than casting it.

This does not need to be changed in this PR though, we can refactor later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please suggest the change. I cannot understand how shoudl I change that

Copy link
Contributor

Choose a reason for hiding this comment

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

I will add a suggestion as a future ticket

@ihardz ihardz merged commit 5c07370 into 901-sveltekit-pod-issues-inline-filters Nov 29, 2022
@ihardz ihardz deleted the 901-sveltekit-pod-issues-missing-filters branch November 29, 2022 23:05
ihardz added a commit that referenced this pull request Nov 29, 2022
* [sveltekit-pod] Issues Search: inline filters (open/closed)

* Code review fixes

* Code review + Components cleanup

* feat: [sveltekit pod] missing issues  filters (#979)

* Dinamic filters: Milestone filter. Dropdown adjustments

* fix label

* Milestone filter adjustments

* Code review fixes

* feat: [sveltekit-pod] sveltekit pod issues pulls route redirect (#998)

* Dinamic filters: Milestone filter. Dropdown adjustments

* fix label

* Milestone filter adjustments

* resolve redirect for issues search pages

* adjust linked components

* adjust linked components

* Code review fixes

* Code review

* Code review

* Code review fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix Filters For Pull Requests And Issues
2 participants