Skip to content

Conversation

@ashwin95r
Copy link
Contributor

@ashwin95r ashwin95r commented Jul 20, 2016

This change is Reviewable

@coveralls
Copy link

coveralls commented Jul 20, 2016

Coverage Status

Coverage decreased (-0.2%) to 52.39% when pulling 12b3a61 on feature/rocksmem into dd7badb on develop.

@manishrjain
Copy link
Contributor

Reviewed 12 of 12 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


posting/lists.go, line 175 [r1] (raw file):

}

func checkMemoryUsage(stores []*store.Store) {

This feels a bit ugly approach. Try to avoid having to generate a list of store pointers.


Comments from Reviewable

@ashwin95r
Copy link
Contributor Author

Review status: 2 of 7 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


posting/lists.go, line 175 [r1] (raw file):

Previously, manishrjain (Manish R Jain) wrote…

This feels a bit ugly approach. Try to avoid having to generate a list of store pointers.

Modified the way the stores are passed as discussed.

Comments from Reviewable

@manishrjain
Copy link
Contributor

Looks much better now! :lgtm:


Reviewed 5 of 10 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@manishrjain
Copy link
Contributor

:lgtm:


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@ashwin95r
Copy link
Contributor Author

Merged to develop with commit 487420a

@ashwin95r ashwin95r closed this Aug 3, 2016
@pawanrawal pawanrawal deleted the feature/rocksmem branch August 30, 2016 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants