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

feature/ATC-77 #118

Merged
merged 14 commits into from
Nov 25, 2016
Merged

feature/ATC-77 #118

merged 14 commits into from
Nov 25, 2016

Conversation

n8rzz
Copy link
Owner

@n8rzz n8rzz commented Nov 18, 2016

completes #77

Adds initial code for ModelSourceFactory and ModelSourcePool

@n8rzz n8rzz added this to the v3.2.0 milestone Nov 18, 2016
@n8rzz
Copy link
Owner Author

n8rzz commented Nov 18, 2016

@erikquinn I'm about half way through this one but I think there is enough here for you tho get an idea of where this is headed. This one might blow your hair back a little bit 😉 , so ask questions on the stuff that doesn't make sense.

@n8rzz n8rzz changed the title [WIP] feature/ATC-77 feature/ATC-77 Nov 19, 2016
@n8rzz
Copy link
Owner Author

n8rzz commented Nov 19, 2016

@erikquinn this is as good as it's going to get before I finish #53. I added issues #120 and #121 for future work that can't be completed now, mostly because of #53.

Take a look and give this a test drive, I want to make sure I didn't break anything. My tests show that I haven't, but I like to be sure.

@n8rzz n8rzz removed the WIP label Nov 20, 2016
arguments for creating a fully formed modelSource instance, eliminating
the need for the caller to also run .init() once a modelSource has been
obtained.
Copy link
Collaborator

@erikquinn erikquinn left a comment

Choose a reason for hiding this comment

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

I'm not sure if it's related to this specifically or not, but the only weirdness I noticed was that upon rerouting an aircraft (in this case from BSR.BSR2.KSFO to QUINN.BDEGA2.KSFO, they will climb of their own accord to very high altitude and not come down. They can be told to descend normally, but a strange altitude is being put in there. May or may not be related.

That aside, everything relating to fixes seems to be working properly, which, as I saw it in review, is the only place the reusability factory is put to work.

* Remove a reusable from the collection and return it for use by the caller
*
* @for ModelSourcePool
* @method releasModelFromPool
Copy link
Collaborator

Choose a reason for hiding this comment

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

releaseReusable

Copy link
Owner Author

Choose a reason for hiding this comment

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

done

I'm so glad I have you look over these PRs for me.

* @private
*/
_removeItem(model) {
this._items = _without(this._items, model);
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 that's a neat lodash function... it will come in handy 😄

Copy link
Owner Author

Choose a reason for hiding this comment

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

And it doesn't mutate _items it returns an entirely new array.

@n8rzz
Copy link
Owner Author

n8rzz commented Nov 25, 2016

@erikquinn added #155 for the bug you reported. It may be something, it may not, but at least we have visibility.

@n8rzz n8rzz merged commit 6f1c6b9 into develop Nov 25, 2016
@n8rzz n8rzz deleted the feature/ATC-77 branch November 25, 2016 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants