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

Added the dev script setup #30

Closed
wants to merge 8 commits into from

Conversation

evalott100
Copy link
Collaborator

Fixes #29

@DominicOram I added the dev environment script, This comes with a few warnings (e.g it wants a pyproject.toml and Distutils is imported before setup tools). Code tests pass, and tickit devices run fine. Idk if you want to fix the warnings here or make it a separate PR?

@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #30 (10e32ca) into main (2461275) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #30   +/-   ##
=======================================
  Coverage   84.30%   84.30%           
=======================================
  Files           3        3           
  Lines         172      172           
=======================================
  Hits          145      145           
  Misses         27       27           
Impacted Files Coverage Δ
tickit_devices/synchrotron/synchrotron_current.py 89.36% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

This looks good, can we:

Comment on lines +24 to +25
- name: Checkout Artemis
uses: actions/checkout@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

Should: This isn't artemis

Comment on lines +7 to +8
# Run every Monday at 8am to check latest versions of dependencies
- cron: "0 8 * * WED"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should: This comment disagrees with the cron job

@abbiemery
Copy link
Collaborator

@evalott100 This may be solved now by #32

@evalott100
Copy link
Collaborator Author

@abbiemery Yep, all it would be adding is the .sh to do the pip venv installation which I think is unnecessary anyway. Closing.

@evalott100 evalott100 closed this Feb 21, 2023
@abbiemery abbiemery deleted the 29_use_a_pip_venv_instead_of_pipenv branch February 28, 2023 14:07
@abbiemery abbiemery restored the 29_use_a_pip_venv_instead_of_pipenv branch February 28, 2023 14:07
@abbiemery abbiemery deleted the 29_use_a_pip_venv_instead_of_pipenv branch February 28, 2023 14:08
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.

Use a pip .venv instead of pipenv
3 participants