Updating Continuous Integration using GitHub Actions and VSCode settings

HERMES-SOC / hermes_instrument

A Python package template for hermes packages

Other

3 stars 4 forks source link

Updating Continuous Integration using GitHub Actions and VSCode settings #3

Closed ehsteve closed 2 years ago

ehsteve commented 2 years ago

This pull request cleans up and expands the continuous integration so that it is easier to see where problems arise

Testing workflow: all testing is now done on multiple platforms and multiple versions of python on each platform.
Formatting and Linting workflow: uses flake8 and black and specifies which patterns to use. The list is copied from the sunpy project. Only tested on Python 3.8 and linux.
Building docs: Tests whether docs can be successfully built using sphinx. Only tested on Python 3.8 and linux.

This PR also includes VSCode settings to ensure that the dev environment is helping to ensure that tests do not fail. It uses black and flake8 are installed in the environment.

ehsteve commented 2 years ago

@tonyastrofig Could you make sure that these VSCode settings work properly on your system please?

tonyastrofig commented 2 years ago

@ehsteve sure I'll see if they work. I assume the best way to do that Is "Merge pull request" and then rerun the docker?

ehsteve commented 2 years ago

Don't merge! You can check out this branch and get it on your machine for testing. Let me know if you don't know how to do that.

tonyastrofig commented 2 years ago

Glad I asked. Yep, I know how to check out the branch. I'll give it a try. Thanks.

ehsteve commented 2 years ago

I see an issue that the vscode settings are set both in the .vscode folder as well as the .devcontainer. @tonyastrofig would mind investigating this?

tonyastrofig commented 2 years ago

Yes, I'll look into the vscode settings in both folders and see which is preferable.

tonyastrofig commented 2 years ago

I'm continuing to research, but it seems like the vscode settings should all be in .devcontainer (since we're running in the container) and should be removed from .vscode. I think that the .vscode settings.json file is being ignored right now but still looking into it.

ehsteve commented 2 years ago

@tonyastrofig could you look into #2? I can't use docker on my windows machine yet. Might as well fix #3. I'll go ahead and do that.

ehsteve commented 2 years ago

@tonyastrofig I fixed number 3.

tonyastrofig commented 2 years ago

@ehsteve I just pulled the latest and the Docker file runs with #1 (pylance) and #3 (deprecated terminal.integrated) both fixed. And deleting the .vscode settings file didn't have any negative side effects.

tonyastrofig commented 2 years ago

@ehsteve The windows message from my comment #2 continues to appear. Based on some internet searches it seems to be indicating that accessing files on the Windows host file system from a Linux container will perform slower than accessing files that are already in a Linux filesystem, similar to accessing files on a remote file share. The recommended fix is to "store your source files in a WSL2 distro's file system (which you can bind mount to the container) or building your container image to include all the files needed rather than storing your files in the Windows file system."

As I'm not fluent in Docker, I'm not entirely sure which part of our setup is accessing Windows files. Any thoughts?

ehsteve commented 2 years ago

That sounds like we can ignore this. This is how vscode works. It lets you work on files on your windows machine while running inside a container. Just curious but what is issuing this message? Docker?

ehsteve commented 2 years ago

If you are okay with it please update your review so we can merge.