Add devcontainer #1571
Add devcontainer #1571
Conversation
|
I would also exclude the |
|
Hi, thanks for that! But can you provide a bit more rationale of why this is useful in our repo? E.g is this only to improve the development of the extension itself? (also, can we not update yarn version in this same PR, I'd rather do it separately) |
yes, this only supports and improves the development of the extension itself by providing every developer who wants to contribute with the same docker-based, reproducible environment without the hassle of following any required setup instructions like an using and old yarn version. |
There was a problem hiding this comment.
Sorry for slow review, left a few of comments.
I'm still figuring out the contribution process - Databricks license is not a standard open source one, and we require all contributors to sign a Contributor License Agreement. I'll let you know how to do that soon.
| spec: "@yarnpkg/plugin-interactive-tools" | ||
| - path: .yarn/plugins/@yarnpkg/plugin-version.cjs | ||
| spec: "@yarnpkg/plugin-version" | ||
| - path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs |
There was a problem hiding this comment.
Let's revert changes here too? Also in the package.json and yarn.lock files
There was a problem hiding this comment.
The file is formatted according to prettier, so I'd argue, that it should stay this way.
The changes in package.json and yarn.lock are produced by the defined yarn version and the specified databricks-sdk version, so I'd also argue, that these are also correct
There was a problem hiding this comment.
If you want to remove the hash from the packageManager field, please provide, how you would like to install yarn, as this came from corepack
There was a problem hiding this comment.
We have the setup guide in the readme here: https://github.com/databricks/databricks-vscode?tab=readme-ov-file#getting-started
Installing the yarn (and sdk) that way doesn't result in different sha, I'd prefer to keep it that way, at least in the scope of this PR.
Sorry for being nit picky, I'm just not comfortable with accepting changes that modify dependency versions (and their hashes) without obvious (to me) reasons.
There was a problem hiding this comment.
no worries, I'll fix this and setup a different pr for a possible switch to corepack and yarn v4
There was a problem hiding this comment.
The referenced instructions define v2 while the package.json defined v3.2.1. which one is correct? I assume the later one? but that one cannot be installed via npm
|
I've figured out the process: we need to send you our CLA to sign before we can accept this contribution. Can you please provide your name, email address, and job title. If you are ok with that, please send an email with the info to [email protected] email, and we will take it from there. |
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
Changes
In order to supply a standardized development environment for further development, I decided to add a devcontainer.
This also updates yarn to the latest stable version, as the mentioned yarn version was released almost exactly 3 years ago.
Fix #1572
Tests
Tested manually