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

Windows support #60

Merged
merged 3 commits into from
Oct 11, 2023
Merged

Windows support #60

merged 3 commits into from
Oct 11, 2023

Conversation

zmalkmus
Copy link
Collaborator

@zmalkmus zmalkmus commented Oct 5, 2023

Added support for Windows OS with the agvis run command.

@zmalkmus zmalkmus self-assigned this Oct 5, 2023
@jinningwang
Copy link
Member

It works well on my Windows laptop, awesome!

One minor concern is that, I noticed that when running command agvis run, it required to be in the path like this "C:\Users\jwang175\agvis\agvis". Is it possible to do some minor adaption that alleviate this so that user can run agvis regardless of the current directory? This might be done in the future as another PR. See andes_root implementation for more information.

The reasons are that:

  1. this is what we do in ANDES and AMS, so it will be more consistent
  2. command agvis can be run regardless current path, so it will be more intuitive to also allow agvis run

@zmalkmus
Copy link
Collaborator Author

zmalkmus commented Oct 5, 2023

I believe I can make app.py do this by setting the path variable.

@jinningwang
Copy link
Member

I believe I can make app.py do this by setting the path variable.

Shall I merge this PR and then we do the path variable later or wait for it in this PR?

@zmalkmus
Copy link
Collaborator Author

zmalkmus commented Oct 7, 2023

I believe I can make app.py do this by setting the path variable.

Shall I merge this PR and then we do the path variable later or wait for it in this PR?

It shouldn’t take long to do so I think it is fine to wait until I figure it out.

@zmalkmus
Copy link
Collaborator Author

Jinning, I have now made agvis run start the server from the app.py directory. Let me know if this change works on your machine and we can merge.

@jinningwang
Copy link
Member

Jinning, I have now made agvis run start the server from the app.py directory. Let me know if this change works on your machine and we can merge.

It works well on my machine, thanks!

@jinningwang jinningwang merged commit 8f69adc into develop Oct 11, 2023
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.

2 participants