-
Notifications
You must be signed in to change notification settings - Fork 214
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
Initial version of a devcontainer for Verible #2068
base: master
Are you sure you want to change the base?
Initial version of a devcontainer for Verible #2068
Conversation
@hzeller : Can you review and consider approving? |
# Available versions: https://github.com/devcontainers/images/tree/main/src/cpp | ||
FROM mcr.microsoft.com/devcontainers/cpp:ubuntu-22.04 | ||
RUN apt update && apt install -y clang-format clang-tidy | ||
RUN wget https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-amd64 -O /usr/local/bin/bazel && chmod a+rx /usr/local/bin/bazel && /usr/local/bin/bazel --version |
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.
Can we fix bazel version to 5.x.x?
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.
This specific code uses bazelisk to install the latest version of bazel. I didn't see the GitHub workflow for the repo doing something to obtain a 5.x.x. version, but may have missed it. Does the workflow do something specific?
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.
Ok, using a .bazelversion file seems to be the way to go, I'll do that in a separate PR. FYI, the devcontainer will not work today without it since verible is not yet compatible with bazel 7.0.0
.devcontainer/devcontainer.json
Outdated
"name": "Verible Dev", | ||
"build": { | ||
"dockerfile": "Dockerfile", | ||
// Add any proxy settings if the build of the |
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.
Please remove commented JSON about proxy settings
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've moved the details to the top of the file. Please comment if this meets the crux of your feedback.
@corco: Please let me know if there are any other changes needed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2068 +/- ##
=======================================
Coverage 92.95% 92.95%
=======================================
Files 358 358
Lines 26446 26446
=======================================
Hits 24583 24583
Misses 1863 1863 ☔ View full report in Codecov by Sentry. |
# Available versions: https://github.com/devcontainers/images/tree/main/src/cpp | ||
FROM mcr.microsoft.com/devcontainers/cpp:ubuntu-22.04 | ||
RUN apt update && apt install -y clang-format clang-tidy | ||
RUN wget https://github.com/bazelbuild/bazelisk/releases/download/v1.19.0/bazelisk-linux-amd64 -O /usr/local/bin/bazel && chmod a+rx /usr/local/bin/bazel && /usr/local/bin/bazel --version |
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.
Ok, using a .bazelversion file seems to be the way to go, I'll do that in a separate PR. FYI, the devcontainer will not work today without it since verible is not yet compatible with bazel 7.0.0
Hi @corco : I don't have write access to the repo, so I can't merge this PR. Can you please merge it or grant me write access? Thanks. |
Can you add a README to the directory that explains what this is for possibly with links pointing to documentation ? |
Please add the environment variable |
Recent changes should make it possible to compile with Bazel 7, so maybe the Since I can't test the devcontainer stuff I can't test that myself. Passing on to @corco (who can have a look next week when back) @kbrunham-intel : can you add a README in the directory briefly explaining what it is for ? |
Initial version of the .devcontainer/ directory and configuration.
This devcontainer uses a Dockerfile based on the Microsoft CPP container, augmented to install Bazel, clang-format, and clang-tidy.
With the devcontainer, developers can clone the repo, and open it within a standard container definition.