Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DEV: Add gitpod files #48107
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
DEV: Add gitpod files #48107
Changes from 10 commits
ca20736
7fe67e6
1c8929f
4b70c74
e527210
1795077
4fd5cdd
bdbc11a
82e771e
540b144
f3beca0
424b199
38e096e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this common to add for gitpod? I don’t think all of our developers use VSCode so am somewhat hesitant to have a say on what “good” extensions are
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might rather want to look at contributors that do use VSCode, and see which extensions those typically use, to ensure we can give a good base experience here (if you don't use VSCode, you also won't have expectations about which extension should be enabled, so as long we provide a useful set, those users will be happy I think)
In any case I suppose we want to enable some extensions. For example at least the python extension? Further of course it's a bit the question how far we want to go (eg restructuredtext highlighting can be useful since our docs use that a lot, maybe cython highlighting could be added as well)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these seem useful, but I'd remove
autodocstring
as docstrings in pandas are often created via decoratorsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we even need to use conda / mamba within Docker? Wondering if it’s worth having an extra layer of virtualization versus simplification not going through all these steps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Maybe related), if we are not expecting users to be able to alter the dev environment when using
gitpod
, maybe mamba/conda can be removed entirely? https://pythonspeed.com/articles/conda-docker-image-size/There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want to keep using conda / mamba, another option could also be to use micromamba, which avoids having an additional base environment (the second point mentioned in the linked blog post for why the resulting docker images become big)
And we should probably also clean up the cached downloaded packages with conda.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking we could simply pip install from requirements.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW I put some comparisons on the mamba within Docker versus without in #49981
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that's useful. I will comment about the usage of mamba there further.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensuring this is done in a single step makes this more efficient because then the conda clean also happens directly in the same layer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't be opposed to adding this to
environment.yml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is depth of 12 then deep enough? Or will this still automatically fetch tags? (otherwise you can also explicitly fetch tags in a separate command)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it should be when we last chatted about this?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
esbonio is not in the list of extensions in gitpod.yml, would that be needed? (or those settings removed) Or is this a default extension?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add those aliases? I can imagine that those will typically different for contributors (eg I use
git s
instead ofgit st
for status ;))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this could be quite user-specific, so at first, I thought we sould remove them.
But on second thought, I can see how for very new users, it can be useful to put something in for sprints. They can be quickly introduced to using aliases without learning how to set them, and more experienced users can over-write them easily by setting their own aliases, no?!
WDYT, keep or remove?