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

Ensemble-problem compatible update #13

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

StuartBenjamin
Copy link

No description provided.

Copy link
Contributor

@henrikjaerleblad henrikjaerleblad left a comment

Choose a reason for hiding this comment

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

I am basically ready to approve. It would be nice with some documentation however?

src/utils.jl Outdated Show resolved Hide resolved
src/orbit.jl Show resolved Hide resolved
@lstagner
Copy link
Collaborator

lstagner commented May 3, 2022

Feel free to only use the new return error codes instead of keeping the old stuff for backwards compatibility

Copy link
Contributor

@henrikjaerleblad henrikjaerleblad left a comment

Choose a reason for hiding this comment

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

Looks good.

@lstagner
Copy link
Collaborator

lstagner commented Jun 6, 2022

Let me know if you are happy with this and I'll go ahead and merge it.

@StuartBenjamin
Copy link
Author

Hi Luke and Henrik, thanks for this. I'll wait to ask you to merge until I've finished working on the orbit tomography package, in case I make any more changes to this one:)

@henrikjaerleblad
Copy link
Contributor

Sounds good! Just let us know when it's all ready. I would like to review OrbitTomography.jl further, but I consider GuidingCenterOrbits.jl ready for merge whenever now.

Project.toml Outdated Show resolved Hide resolved
src/utils.jl Show resolved Hide resolved
src/orbit.jl Show resolved Hide resolved
src/utils.jl Show resolved Hide resolved
@henrikjaerleblad
Copy link
Contributor

henrikjaerleblad commented Jul 8, 2022

I am ready to approve. @lstagner could you take one quick look-through as well?

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