Skip to content
This repository was archived by the owner on Jul 30, 2018. It is now read-only.

Conversation

maier49
Copy link
Contributor

@maier49 maier49 commented Nov 7, 2016

Type: cleanup
Description:

Resolves #39 by simplifying the Query type to only have one generic. This depends on the changes in #32 and will need to be merged after it.

Related Issue: #39

Please review this checklist before submitting your PR:

  • There is a related issue
  • All contributors have signed a CLA
  • All code matches the style guide
  • The code passes the CI tests
  • Unit or Functional tests are included in the PR
  • The PR increases or maintains the overall unit test coverage percentage
  • The code is ready to be merged

@dylans dylans added this to the 2016.11 milestone Nov 9, 2016
@maier49 maier49 force-pushed the remove-second-generic-on-query branch from 78c9e28 to ae3649b Compare November 16, 2016 17:46
@codecov-io
Copy link

codecov-io commented Nov 16, 2016

Current coverage is 98.08% (diff: 100%)

Merging #43 into master will decrease coverage by 0.01%

@@             master        #43   diff @@
==========================================
  Files            17         17          
  Lines          1002        993     -9   
  Methods           6          6          
  Messages          0          0          
  Branches        177        177          
==========================================
- Hits            983        974     -9   
  Misses            1          1          
  Partials         18         18          

Powered by Codecov. Last update 8cffa6f...c5b9eca

@maier49 maier49 mentioned this pull request Dec 3, 2016
7 tasks
@dylans dylans modified the milestones: 2016.11, 2016.12 Dec 5, 2016
@maier49 maier49 force-pushed the remove-second-generic-on-query branch from b2e2b5b to bd70bf6 Compare December 5, 2016 17:26
Copy link
Member

@agubler agubler left a comment

Choose a reason for hiding this comment

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

A couple of minor comments

@@ -94,7 +94,7 @@ function createFilterHelper<T>(filters: FilterChainMember<T>[], serializer: (fil
return data.filter(this.test);
},
filterChain: filters,
toString(this: Filter<T>, filterSerializer?: ((query: Query<any, any>) => string) | ((filter: Filter<T>) => string)) {
toString(this: Filter<T>, filterSerializer?: ((query: Query<any>) => string) | ((filter: Filter<T>) => string)) {
Copy link
Member

Choose a reason for hiding this comment

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

can the Query type use T ? (query: Query<T>) => string)

toString(querySerializer?: (query: Query<any, any>) => string): string;
export interface Query<T> {
apply(data: T[]): T[];
toString(querySerializer?: (query: Query<any>) => string): string;
Copy link
Member

Choose a reason for hiding this comment

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

Again T?

@maier49 maier49 merged commit 3b71ac1 into dojo:master Dec 12, 2016
@maier49 maier49 deleted the remove-second-generic-on-query branch December 12, 2016 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants