-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Conversation
Remove unnecessary package
swift-format
There are quite a lot of changes in here. Not sure we need to include all of the extensions in here. Plus I think it is worthwhile bringing in @vknabel to discuss if there should be a transition from the original swift extension or should we add a new swift devcontainer for the SSWG extension. I think this is also a good time to discuss what is actually needed in the devcontainer. For me there appears to be a lot of unnecessary stuff here. Why are we installing node, python? Is this something MS expects? Including @0xTim |
We can remove the lldb install for sure though, it is overriding the swift version of lldb. |
I'm guessing node is needed to run the VSCode environment. Python was probably installed for LLDB or to install Swift, but that's now been removed. As for all the extra extensions - I agree that we're installing a lot we probably don't need. Sticking to just |
You don't need node.js, well at least I can build a devcontainer which doesn't install it. Could it be included in the swift docker images already? We don't need to include vscode-lldb as the swift extension automatically installs it. |
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.
As I've said in previous comments, can we keep this to just the sswg.swift-lang
extension. Plus we need to work out what we want to do with the other swift vscode extension.
"sde.languageservermode": "sourcekite", | ||
"swift.path.sourcekite": "/usr/local/bin/sourcekite" | ||
"lldb.library": "/usr/lib/liblldb.so", | ||
"sourcekit-lsp.toolchainPath": "/usr/bin/", |
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.
You shouldn't need to setup sourcekit-lsp paths
@@ -14,15 +14,8 @@ ARG USER_GID=$USER_UID | |||
COPY library-scripts/common-debian.sh /tmp/library-scripts/ | |||
RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \ | |||
&& /bin/bash /tmp/library-scripts/common-debian.sh "${INSTALL_ZSH}" "${USERNAME}" "${USER_UID}" "${USER_GID}" "${UPGRADE_PACKAGES}" "true" "true" \ | |||
&& apt-get -y install --no-install-recommends lldb python3-minimal libpython3.7 \ |
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.
👍 swift docker images install a later version of python.
Hi, thanks for the mention. I haven't found time for a specific migration plan, yet. But I do also vote for The few benefits msde still has, are mostly restricted to very old Swift versions and other niche use cases. Both user groups won't use this dev container. Regarding the other extensions I'd also prefer using In case we wouldn't, we'd need set up and install all formatter extensions (for apple's and nicklockwood's), not just one to stay neutral and not opinionated. And in that case, also add Swiftlint as it's too widespread. IMHO far too much extensions. (Do I remember correctly, Regarding the other extensions: haven't heard of them until now. But to be fair, haven't been active for a while. Slightly OT: msde will either be removed or transformed into an extension bundle. Legitimate features will be split into multiple extensions. The features |
I removed the other extensions, even though they are all quite useful. |
"vknabel.vscode-swift-development-environment", | ||
"vadimcn.vscode-lldb" | ||
"sswg.swift-lang", | ||
"vadimcn.vscode-lldb", |
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 is not required as it's pulled in automatically by sswg.swift-lang
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.
LGTM other than the LLDB requirement as it's pulled in automatically
@tomerd I wonder if there's anyone we can poke about getting this added as an official language rather than a community one |
@@ -16,9 +16,7 @@ | |||
], | |||
|
|||
// Set *default* container specific settings.json values on container create. | |||
"settings": { | |||
"lldb.library": "/usr/lib/liblldb.so", |
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.
@damuellen sorry I may have confused things - this is definitely still required - it's the "vadimcn.vscode-lldb",
line in extensions we don't need
LGTM @0xTim Regarding Official vs Community. We can just edit the .devcontainer to say "Swift" instead of "Swift (community)". I'm not sure if that's affects these changes getting merged. |
@adam-fowler The "Community" indicator is more about whether it's a 1st party maintained definition or not. More of a statement around support and ongoing maintenence than validity. Otherwise, this PR looks great! Happy to merge if you feel like it is ready. |
Well it will be maintained by the Swift Server working group (group promoting swift on server consisting of a mixture of Apple employees and various people from other organisations). If we can't have |
@Chuxel Should I open a new PR with squashed commits ? |
@damuellen Nope, no need - I'll do that at merge time (via "Squash and merge"). @adam-fowler In concept, we could have it say The README should also get updated with the go-forward owner conacts we can "@" in the event an issue or PR is raised. Right now the README says @cloudnull. |
@Chuxel if we can have just In terms of the README @damuellen if you want to update that for Swift, I'm happy to be listed as a maintainer. It doesn't look like the SSWG team can be tagged so you can list me, @adam-fowler if he's happy, yourself if you want and @cloudnull if he still wants to be tagged |
@0xTim Sure I can do this. But I will still wait for the feedback of others. |
Yeah you can put me on there |
@0xTim Yeah this is more about signaling who is maintaining and keeping the images up to date, etc. The implicit assumption ends up being the VS Code team. I'd be happy to change this to something indicating the organization maintaining the image rather than Otherwise, happy to merge. |
@adam-fowler @0xTim lets discuss and finalize on our next meeting |
Hi @bamurtaugh it looks like we will be going forward with hosting our own versions of the swift devcontainer templates. I am away for the next 10 days so will start looking into it once I get back. |
Fantastic, thanks for letting us know, @adam-fowler! |
Hi @bamurtaugh , The template at the moment is pretty much a copy of the one in vscode-dev-containers. It currently has a number of options (install zsh, update packages, install NodeJS) which we may want to change. Are these options expected to be available or do we have free reign to change as we see fit? |
Hi @adam-fowler! Thanks so much for sharing this. The template looks good to me! One note is you could consider renaming the repo to something like "swift-devcontainer" as we're broadening dev containers to any tool with the open spec. I also think the readme looks great - one small change you might consider is that the Remote - Containers extension has been renamed to "Dev Containers," but the link will still work either way. I'm also sharing it in our new community Slack channel for other eyes. We'd love to have you join our community Slack by the way - here's where you can comment if you'd like to join: https://github.com/orgs/devcontainers/discussions/3.
The contents of the template are totally up to your team - please feel free to adjust it as you see fit. As it sounds like these could be breaking changes for users currently using the template, we might recommend putting info in the REAMDE and/or a pinned issue in your repo to notify users about changes. |
Any changes we would make would be heavily publicised. Depending on responses we could also create a legacy container which was a copy of the original. How do I go about publishing this template and will it clash with the currently published one? |
@adam-fowler The starter template has some info on this - https://github.com/devcontainers/template-starter#distributing-templates. You can use a GitHub Action to create the needed OCI Artifact - just copy the You can do a PR to add your template collection to the index so it shows up in UX (https://github.com/devcontainers/template-starter#adding-templates-to-the-index). This is one time setup for the repo, so you can add as a many template vacations as you want at that point and they'll all show up. We can then remove the one here from the index just by deleting the devcontainer-template.json file so there's no conflict. |
@Chuxel Unfortunately Apple won't allow the usage of the GH action. So I need to do the OCI Artifact generation and release manually |
Hey @adam-fowler, happy new year! I just wanted to check in - please let us know if you needed any help with releasing your Template, we're happy to help. |
Hi, I just haven't got around to it. I'll see if I can find time this week. |
@bamurtaugh having looked through all the documentation and played with the devcontainer cli tool can I just use
|
@adam-fowler, the Which container registry are you going to use? |
@bamurtaugh I hadn't really thought about that. I assume we would use the |
@bamurtaugh can we use hub.docker.com? |
Yes! You can distribute using any OCI registry that implements the OCI Artifact Distribution Specification: https://containers.dev/implementors/templates-distribution/#oci-registry. |
Hi, we're back to publishing in ghcr.io. First I logged into ghcr.io
Then I tried to publish the template using
But am getting the following error
I have also tried namespace |
Hi @adam-fowler 👋
Would it be possible to append |
Here you go
|
Right now there are several ways to provide registry credentials to the CLI. Unfortunately we don't yet have support for reading from OS credential helpers (like what is probably being used on your Mac when you do that Here's the code that looks for the credentials: The quickest way for you to get publishing from your local machine should be to do:
|
Other auth options include:
|
bingo, looks like that worked |
Awesome! Making a note to document this more clearly today. |
@bamurtaugh Here is the PR for adding the Swift dev container devcontainers/devcontainers.github.io#150. I hope it all works. At the moment it looks like the old devcontainer, but we hope to update to the newer devcontainer feature style that all the new containers seem to have moved to. |
Thanks, looks good to me! ✨ Have #1713 opened to deprecate the older Template. |
Nothing happens when I select either of these. I'm currently on a really bad internet connection so don't know if that is the cause. |
Thanks so much for the PR, @adam-fowler! This is really exciting, and we really appreciate your collaboration. I'm able to add the Swift Template successfully in the Dev Containers extension. If you're able to 1) try again as you get a more stable internet connection, 2) let us know if Swift is the only Template this is happening for (suggesting there is something unique there for some reason), that'd be great. |
I've tested this and it's working for me so I think we're all good! |
Looks like it's working now |
* Add docs for `devcontainer templates publish` ref: microsoft/vscode-dev-containers#1238 * Update publish.md * Update publish.md * Update docs/templates/publish.md Co-authored-by: Samruddhi Khandale <[email protected]> * Update publish.md --------- Co-authored-by: Samruddhi Khandale <[email protected]>
Changing the setting and installed extension. To reflect the latest state of Swift development.
introducing-swift-for-visual-studio-code