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

feat: Add devcontainer config #2250

Merged
merged 2 commits into from
Jul 15, 2023
Merged

feat: Add devcontainer config #2250

merged 2 commits into from
Jul 15, 2023

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Jul 14, 2023

Description

@crazy4pi314, @guenp, and @ltalirz also gave the SciPy 2023 tutorial Meet your coding best friend: VS Code💖 - A hands-on tutorial on how to get the most out of the world’s most popular Python editor which covers dev containers in it as well: https://github.com/crazy4pi314/scipy-vscode-tutorial

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Add devcontainer config using the pyhf Dockerfile.
   - c.f. https://containers.dev/
   - Motivated by Sarah Kaiser's SciPy 2023 lightning talk.

* Add devcontainer config using the pyhf Dockerfile.
   - c.f. https://containers.dev/
@matthewfeickert matthewfeickert added feat/enhancement New feature or request Docker Involving Docker images or builds SciPy Conference Resulted from discussions at a SciPy conference need-to-backport tmp label until can be backported to patch release branch labels Jul 14, 2023
@matthewfeickert matthewfeickert self-assigned this Jul 14, 2023
@matthewfeickert
Copy link
Member Author

👋 @crazy4pi314 following your great ⚡ talk today and that things seemed pretty simple in bluesky/tiled#522 I wanted to try to get this going for pyhf as well. I'll read https://containers.dev/ but if you also have time to discuss dev containers (or review this PR) before you leave SciPy 2023 that would be great! :)

@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (4845a15) 98.30% compared to head (8daec08) 98.30%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2250   +/-   ##
=======================================
  Coverage   98.30%   98.30%           
=======================================
  Files          69       69           
  Lines        4534     4534           
  Branches      802      802           
=======================================
  Hits         4457     4457           
  Misses         45       45           
  Partials       32       32           
Flag Coverage Δ
contrib 97.88% <ø> (ø)
doctest 61.09% <ø> (ø)
unittests-3.10 96.31% <ø> (ø)
unittests-3.11 96.31% <ø> (ø)
unittests-3.8 96.33% <ø> (ø)
unittests-3.9 96.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment on lines 19 to 20
"GitHub.codespaces",
"ms-vscode-remote.remote-containers"
Copy link

Choose a reason for hiding this comment

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

I'll let @crazy4pi314 give the final answer here, but my understanding is that those extensions are not actually necessary here, unless you want to connect to a remote container / codespace from inside the dev container (of course the extensions should not hurt either).

Copy link
Member Author

@matthewfeickert matthewfeickert Jul 14, 2023

Choose a reason for hiding this comment

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

unless you want to connect to a remote container / codespace

While the pyhf dev team doesn't actively use Codespaces at the moment (very cool, but not clear if the cost is worth it atm for us) we would like for others to be able to use Codespaces or Gitpod for demos and to help people making contributions get onboarded. Do these extensions help / become needed there?

Copy link

@crazy4pi314 crazy4pi314 Jul 15, 2023

Choose a reason for hiding this comment

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

Yeah I think they are probably not needed, Its likely my fault that I was exporting envs and those got installed as needed (but you don't have to proactively).

Gitpod I think has some very inital support for dev container config, so I am not sure if they would be helpful there yet :)

@matthewfeickert matthewfeickert marked this pull request as ready for review July 15, 2023 19:51
@matthewfeickert matthewfeickert merged commit 9797a21 into main Jul 15, 2023
18 checks passed
@matthewfeickert matthewfeickert deleted the feat/add-devcontainer branch July 15, 2023 19:59
@matthewfeickert
Copy link
Member Author

Thank you @crazy4pi314 and @ltalirz! 🚀

@matthewfeickert matthewfeickert removed the need-to-backport tmp label until can be backported to patch release branch label Aug 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docker Involving Docker images or builds feat/enhancement New feature or request SciPy Conference Resulted from discussions at a SciPy conference
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants