Jasmine refactor/backfill command/creating collections service2 #974
Conversation
…ommand to use the service.
|
You're completely right. There is duplicate code all over the place and it's very hard to refactor to achieve testability. Also having functions declared inside of functions is bad. It's very hard to understand the code flow in the project's current state. |
lib/_codemap.js
Outdated
| 'slow_stochastic': require('./slow_stochastic') | ||
| 'slow_stochastic': require('./slow_stochastic'), | ||
|
|
||
| 'collection-service': require('./services/collection-service') |
There was a problem hiding this comment.
I think, "/services/collection-service'" should be named "/services/collection". No need to duplicate the "service". What do you think?
There was a problem hiding this comment.
True, there's no need, still I think it adds more than it hurts.
|
@JensvdHeydt no, I do not. I've been thinking about moving it to a fork I control, but that's the "nuclear" option |
|
@tiagosiebler reached out to github support, they've been helpful in the past, maybe they have a solution for this too. |
…llectionsService2
|
They've given me a new API endpoint that will allow me to migrate all issues. I've created #1010 to track and get more opinions. |
…aVir#974) * Created collection-service.js, and a unit test. Refactored backfill command to use the service. * Added jasmine.js
So, this commit shows where I'm going with unit tests. There is a lot of duplicate functionality across the commands, strategies, and exchanges. I want to DRY that up, using one service function for each place we currently call that duplicate functionality.
To that end, I've created collection-service.js, and called it from the command backfill.js. Also, to the point of this whole effort, I created a unit test for the collection service.