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

Just being honest #20713

Merged
merged 19 commits into from Feb 19, 2018
Merged

Just being honest #20713

merged 19 commits into from Feb 19, 2018

Conversation

islemaster
Copy link
Contributor

Removing a number of TODOs that, let's be honest, I'm never going to do anything about.

giphy 1

...with setting an editor enum or (even better) inject an appropriate editor-adaptor.
...for a Droplet bug,
See droplet-editor/droplet#137
Calling getValue() updates the cached ace editor value, which can be out-of-date in droplet and cause an incorrect early-out.
Could remove this line once that bug is fixed and Droplet is updated.
...the game lab level object properties.
...Eventually, we ought to preview and get dimensions from the local filesystem, async with the upload itself, but that will mean refactoring away from the jQuery uploader.
...when we have a shared Javascript User object that can be available on page load.
...more likely to ensure it's unique and fits within 48 characters.
Maybe grab this MIT-licensed implementation via node?
https://github.com/blueimp/JavaScript-MD5
Do we benefit from inheritance here?  Would it be cleaner to make this not-an-entity that manipulates a stock NetSimClientNode?  Will another developer find it easy to understand how this class works?
...instead of making two in this method.
...and ticking the routers up to netsim.js (or elsewhere)
...so we can be notified when writes succeed instead of making a 50ms guess, and make this a properly async method.
...in the global namespace for testing purpose. Would be nice to eliminate this eventually.
...that's happening in all apps?
Copy link
Contributor

@caleybrock caleybrock left a comment

Choose a reason for hiding this comment

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

Fine by me. I like that you have a br-pair name 😂

islemaster added a commit that referenced this pull request Feb 19, 2018
@islemaster islemaster merged commit 48a99d0 into staging Feb 19, 2018
@islemaster islemaster merged commit 8a70924 into staging Feb 19, 2018
@islemaster islemaster deleted the brad-todos branch February 19, 2018 18:17
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.

None yet

3 participants