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

move functionality from Javis to JavisViewer #2

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

gpucce
Copy link
Member

@gpucce gpucce commented Jan 22, 2022

This PR is meant to move all the interactive and liveviewing parts of Javis to JavisViewer

Closes:
#1

@gpucce gpucce requested a review from TheCedarPrince January 22, 2022 12:27
@gpucce
Copy link
Member Author

gpucce commented Jan 22, 2022

Hi @TheCedarPrince I started working on this, it would be nice if you could merge this PR so we have CI working or give me merge access and I can do it myself.

It is not finished yet but just so we can set up some initial stuff it should be good to go, test locally and it works.

@gpucce
Copy link
Member Author

gpucce commented Jan 22, 2022

I did not know this but since it is not running here CI runs on my fork! So no rush at all!!

@TheCedarPrince
Copy link
Member

Hey @gpucce , you should now have access to all the things!
Sorry about that!
I thought the @JuliaAnimators/maintainers team has at least maintainer rights by default.
Before we merge this, something I noticed was that accidentally included in this PR was the live streaming functionality.
This is quite different from the JavisViewer and needs to not be merged here...
Not sure where to put the streaming functionality just yet but we should at least keep it back inside of Javis.jl itself.

@TheCedarPrince
Copy link
Member

I can give more of a review later - right now, quite tired!

@gpucce
Copy link
Member Author

gpucce commented Jan 29, 2022

I was going to ask you if you wanted to move the streaming as well, that I will move back then!!

@Sov-trotter
Copy link
Member

Sov-trotter commented Jan 29, 2022

I think streaming is pretty much basic so better to keep it in base Javis. In the future I am thinking of having such features out of base(in a JavisExtensions package).

@gpucce gpucce marked this pull request as ready for review February 28, 2022 00:04
@gpucce
Copy link
Member Author

gpucce commented Feb 28, 2022

Hi @TheCedarPrince this should be the JavisViewer package! If you can review it and let me know if smth is missing i will add it!

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.

3 participants