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

Remove seer integration #3940

Merged
merged 3 commits into from
Nov 28, 2019
Merged

Remove seer integration #3940

merged 3 commits into from
Nov 28, 2019

Conversation

tsherif
Copy link
Contributor

@tsherif tsherif commented Nov 27, 2019

No description provided.

@coveralls
Copy link

coveralls commented Nov 27, 2019

Coverage Status

Coverage increased (+2.5%) to 83.907% when pulling 8f6ab0d on remove-seer into 150ca28 on master.

Copy link
Collaborator

@Pessimistress Pessimistress left a comment

Choose a reason for hiding this comment

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

Also need to remove the mentioning of seer in documentations.

@ibgreen
Copy link
Collaborator

ibgreen commented Nov 27, 2019

I get that cuts are sometimes necessary.

That said it would be cool if there was a plan around the debug support in the frameworks - particularly if that plan made it clear that while some things are being phase out, the ambition is that the overall debug support will be getting better, or at least that maintainers are positive to accepting contributions in certain areas of debug support.

I obviously think strong debug support is valuable for a suite of GPU focused libraries, so here are some simple things that could go into a roadmap:

  • The Stats/StatsWidget combo - an impressive high-potential recent addition to the overall debug support. A lot more could be done here to make it really useful - just a statement that there is ambition or interest in developing this feature would be great.
  • Also the logging functionality in the library could use some work/docs.

More far out ideas:

  • @probe.gl/bench - build more integrated bench-utils for deck and or luma to facilitate creating performance-focused test "benches".
  • Right now a number of cool projects do "visual studio code" editor integrations, some of them quite impressive.
  • One could even explore connecting deck/luma to tracing frameworks like Jaeger, etc. Both on the producing and consuming (visualization) side.

@@ -310,12 +290,10 @@ export default class LayerManager {
if (!oldLayer) {
const err = this._initializeLayer(newLayer);
error = error || err;
initLayerInSeer(newLayer); // Initializes layer in seer chrome extension (if connected)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If we could leave a hook here, something like LayerManager.callbacks.onInitialize, it will allow users to add back similar functionalities using an external module.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we could leave a hook here, something like LayerManager.callbacks.onInitialize, it will allow users to add back similar functionalities using an external module.

I like that proposal! I am playing with the idea of setting up some open source "dot-gl" libraries from Unfolded's side that could contain some of the more experimental code that is being cut from the core libraries, plus some new stuff that we are developing that would be complementary to the existing frameworks.

That way the vis.gl core libraries could go ahead focus on being really small and hardened, but also provide the extensibility hooks so that we can continue to build out more experimental and advanced functionality (and if and when that matures we can always go back and make it official parts of the suite).

@tsherif
Copy link
Contributor Author

tsherif commented Nov 27, 2019

Also need to remove the mentioning of seer in documentations.

Done.

If we could leave a hook here, something like LayerManager.callbacks.onInitialize, it will allow users to add back similar functionalities using an external module.

This could be cool, but I'd rather build a hook API like this around an actual usage so we know it provides something useful.

That said it would be cool if there was a plan around the debug support in the frameworks

Agree that it would be nice to put together a plan around this. I'd love to see things function more in the plugin style like luma.gl/debug so you don't pay any cost for a tool if you're not using it.

@ibgreen
Copy link
Collaborator

ibgreen commented Nov 27, 2019

This could be cool, but I'd rather build a hook API like this around an actual usage so we know it provides something useful.

Well, the way I see it, seer was "actual usage". In my view, seer was actually very useful. And the deleted code shows exactly where hooks were needed...

@tsherif
Copy link
Contributor Author

tsherif commented Nov 27, 2019

What I mean is if we want seer to be supported in this way, we should build and test seer support as a plugin, rather than just adding hooks and hoping everything works out.

@ibgreen
Copy link
Collaborator

ibgreen commented Nov 28, 2019

What I mean is if we want seer to be supported in this way, we should build and test seer support as a plugin, rather than just adding hooks and hoping everything works out.

Yes that makes complete sense. With the caveat that it depends on how much work it would require signing up for, I'd be interested in helping out with that.

@tsherif tsherif merged commit 80ae951 into master Nov 28, 2019
@tsherif tsherif deleted the remove-seer branch November 28, 2019 00:51
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

4 participants