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

Add Dev Container #233

Merged
merged 10 commits into from
Dec 30, 2021
Merged

Add Dev Container #233

merged 10 commits into from
Dec 30, 2021

Conversation

getjump
Copy link
Member

@getjump getjump commented Dec 20, 2021

Dev Container open up a whole new world of possibilities:

  1. It is easy to make dev environment even for frontenders, for whom currently it is hard as stated by Update README so that even frontend developers can run the bot locally #178
  2. You can develop even on microwave by utilizing cloud super powers of Github Codespaces

There are also task and launch target defined for VS Code, so that environment is kinda complete, you just start vs code open dev container from source, click run and boom, everything just works (including debugging, and lint and stuff 🔥)

We can probably continue to improve that environment in the future (like for example when we can use Python 3.10 #221).

Before merge:

  • update README
  • fix make help error
  • put python deps into image to speedup container starting
  • fix .gitignore (ignore whole ./vscode exclude required for devcontainer)

Next iteration: #237

@getjump
Copy link
Member Author

getjump commented Dec 20, 2021

We probably should store our dependencies in image, since they are not updating that frequently to speed up codespaces/local image builds.

Copy link
Collaborator

@C-Pro C-Pro left a comment

Choose a reason for hiding this comment

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

👍

@egregors
Copy link
Collaborator

Looks cool! I'll check this out next week

@egregors egregors self-assigned this Dec 27, 2021
@egregors egregors self-requested a review December 27, 2021 08:05
Copy link
Collaborator

@egregors egregors left a comment

Choose a reason for hiding this comment

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

Looks pretty cool, really. Especially debugging-in-docker and run in one click. Here are a few improvements proposals and we should provide an updated README for frontends.


RUN apt-get -y update && apt-get install -y ffmpeg

# [Optional] If your pip requirements rarely change, uncomment this section to add them to the image.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like include deps into image is a good idea. We change them not so often.

@@ -103,8 +103,6 @@ venv.bak/
# mypy
.mypy_cache/

#vscode
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we can exclude only launch.json and tasks.json from .gitignoge. Personal setting should be ignored.

@getjump
Copy link
Member Author

getjump commented Dec 28, 2021

@egregors I disagree that we should exclude .vscode from repository, IT SHOULD NOT contain personal preferences, it should contain project wise preference see https://github.com/microsoft/vscode/tree/main/.vscode (especially https://github.com/microsoft/vscode/blob/main/.vscode/settings.json) for example.

@C-Pro
Copy link
Collaborator

C-Pro commented Dec 28, 2021

@egregors I disagree that we should exclude .vscode from repository, IT SHOULD NOT contain personal preferences, it should contain project wise preference

Mine does ¯_(ツ)_/¯

@getjump
Copy link
Member Author

getjump commented Dec 28, 2021

@egregors I disagree that we should exclude .vscode from repository, IT SHOULD NOT contain personal preferences, it should contain project wise preference

Mine does ¯_(ツ)_/¯

Can you provide example what's in your project settings?

@C-Pro
Copy link
Collaborator

C-Pro commented Dec 28, 2021

@egregors I disagree that we should exclude .vscode from repository, IT SHOULD NOT contain personal preferences, it should contain project wise preference

Mine does ¯_(ツ)_/¯

Can you provide example what's in your project settings?

"python.pythonPath": "/home/cpro/.pyenv/versions/3.6.6/bin/python",

@egregors
Copy link
Collaborator

@egregors I disagree that we should exclude .vscode from repository, IT SHOULD NOT contain personal preferences, it should contain project wise preference

Mine does ¯_(ツ)_/¯

Can you provide example what's in your project settings?

"python.pythonPath": "/home/cpro/.pyenv/versions/3.6.6/bin/python",

Fair enough. Mine contains pyPath as well. ://

@getjump
Copy link
Member Author

getjump commented Dec 28, 2021

@egregors @C-Pro Well, I don't like absolute paths relative to local system, probably something like pipenv/venv/etc would be better. But there's not much we can do now. Maybe later we can focus on enhancing process of development without Dev Container, but for now I have found a solution to use settings json specific to container - https://github.com/microsoft/vscode-dev-containers/blob/main/containers/go/.devcontainer/devcontainer.json#L17 . So we can safely put .vscode/settings.json into .gitignore

@C-Pro
Copy link
Collaborator

C-Pro commented Dec 28, 2021

BTW I did not set this path manually. It is changed everytime I select interpreter in vscode via dropdown menu on the bottom.

@getjump
Copy link
Member Author

getjump commented Dec 28, 2021

@C-Pro Ye, that is probably not a best place to store interpreter by vscode 🤷‍♂️

@getjump
Copy link
Member Author

getjump commented Dec 29, 2021

  • try to use docker-compose-dev.yml for devcontainer

@egregors it seems like no fix to me currently. We should probably somehow remove last line, where we start bot from dev Dockerfile or something else.

@egregors
Copy link
Collaborator

  • try to use docker-compose-dev.yml for devcontainer

@egregors it seems like no fix to me currently. We should probably somehow remove last line, where we start bot from dev Dockerfile or something else.

Sure, let's talk about it after NY :D :D I believe we could extract it subtask into a separate issue.

Anyway, your fixes looks grate! I gonna test it locally, just to be sure, and will merge it.

@getjump
Copy link
Member Author

getjump commented Dec 29, 2021

@egregors there are a bit work still left, i accidentally commited junk comments, so probably we should merge after NY🌚🌚

@egregors
Copy link
Collaborator

@egregors there are a bit work still left, i accidentally commited junk comments, so probably we should merge after NY🌚🌚

@getjump If you are talking about junk in Dockerfile, I cleaned it already. But if smth else here, and you wanna freeze merge for a time, it's okey.

@getjump
Copy link
Member Author

getjump commented Dec 29, 2021

@egregors Ye that's probably all. I also added new extension by Microsoft, but we can add it later maybe "visualstudioexptteam.intellicode-api-usage-examples"

@egregors
Copy link
Collaborator

@egregors Ye that's probably all. I also added new extension by Microsoft, but we can add it later maybe "visualstudioexptteam.intellicode-api-usage-examples"

So, in this way I gonna merge it now. @getjump could you please drop a bit more details in #237?

@egregors egregors merged commit c803f63 into vldc-hq:master Dec 30, 2021
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