Skip to content
This repository has been archived by the owner on Nov 30, 2023. It is now read-only.

Use correct PowerShell container #29

Merged
merged 2 commits into from
May 7, 2019
Merged

Conversation

TylerLeonhardt
Copy link
Member

@TylerLeonhardt TylerLeonhardt commented May 2, 2019

A lot of Microsoft containers are moving the MCR - PowerShell being one that has already made the switch.

I would recommend checking with the other containers to see if they use MCR - it's easy to check. It shows up in Docker Hub:

https://hub.docker.com/_/microsoft-powershell?tab=description

Full list:
https://hub.docker.com/publishers/microsoftowner

cc @TravisEz13

@Chuxel
Copy link
Member

Chuxel commented May 2, 2019

LGTM - We had moved the .NET Core ones over but had missed this one.

@Chuxel
Copy link
Member

Chuxel commented May 2, 2019

@chrmarti - Did you want to hold for a few days while the initial release goes out before merging?

@chrmarti chrmarti self-requested a review May 3, 2019 07:25
@chrmarti
Copy link
Contributor

chrmarti commented May 3, 2019

@Chuxel Let's keep merging important fixes, but hold back on feature work. 👍

@TylerLeonhardt
Copy link
Member Author

TylerLeonhardt commented May 3, 2019

I'd like to also use this issue as an opportunity to say that it's great to see that PowerShell was included in the dev container templates!

If your team ever has PowerShell questions or wants to consult our team to verify anything about PowerShell and our extension, please don't hesitate to contact me or my PM @SydneyhSmith 😊

The Remote Development feature was highly requested in the PowerShell community.

I heard you folks made an announcement at Pycon which is awesome! - PowerShell Summit NA was this week (yesterday included) and a smaller PowerShell-related conference is today. I think there could have been opportunities for doing the announcement here as well (I could have done it, or Sydney, to not randomize your team).

Anyway, the PowerShell team evangelizes vscode heavily and we recommend it as THE editor to use with PowerShell.

You all are doing a great job, keep it up!

@chrmarti
Copy link
Contributor

chrmarti commented May 6, 2019

Thanks @TylerLeonhardt ! I think we were too focused to land this for considering a broader scope for the announcement.

@egamma
Copy link
Member

egamma commented May 6, 2019

@TylerLeonhardt as @chrmarti mentioned we were fully heads down.

We appreciate your support and eventually it would be best when your team owns the PS container definition.

@@ -3,7 +3,7 @@
# Licensed under the MIT License. See https://go.microsoft.com/fwlink/?linkid=2090316 for license information.
#-------------------------------------------------------------------------------------------------------------

FROM microsoft/powershell
FROM mcr.microsoft.com/powershell

# Install git, process tools
RUN apt-get update && apt-get -y install git procps
Copy link
Member

Choose a reason for hiding this comment

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

One comment, If you cleanup in separate layers, like this, the final image will be clean, but the data will still be in the layer. It's better to cleanup in the same layer.

Copy link
Member

Choose a reason for hiding this comment

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

@TylerLeonhardt I created a PR for your repo to fix this.

TylerLeonhardt#1

@SteveL-MSFT
Copy link
Member

@egamma if you are doing customizations, it doesn't make sense for us to own that as we can't keep up with knowing what customizations you need. Building off our base container which we own makes sense.

@Chuxel
Copy link
Member

Chuxel commented May 6, 2019

@SteveL-MSFT There really aren't any customizations being applied here to speak of. This is a standard powershell container that also includes git. Installig git is not required for VS Code to work, but is included since it allows you to work with source code from inside the container.

Basically, there's a set of utilities you'd probably want if you were editing code inside the container, so the things in the preconfigured list include them... which is often just git.

If you had a powershell image with git, you could drop the Dockerfile entirely with a decontainer.json file that looked like this:

{
	"name": "PowerShell",
	"image": "your-image-name-goes-here",
	"extensions": [
		"ms-vscode.powershell"
	]
}

@SteveL-MSFT
Copy link
Member

@Chuxel adding git is a customization and nothing something we're adding to our default images

@Chuxel
Copy link
Member

Chuxel commented May 6, 2019

@SteveL-MSFT @SydneyhSmith @TylerLeonhardt Gotcha - your definition of customization was slightly different than mine since VS Code itself isn't "customizing" the images per-se with stuff it needs to run. You mean letting customers use a Dockerfile to create their own image -- these effectively act as starter Dockerfiles.

Anyway, entirely up to you guys on the level of support you want to have for the scenario and how much you want to drive to it. To give you an idea, the Java team has adopted the Java definitions and the Python team has adopted the Python ones so they can be sure they line up with the messages they want to send as they drive developers in this direction.

@Chuxel
Copy link
Member

Chuxel commented May 7, 2019

@TylerLeonhardt We are going to be prepping a release soon. Should we merge in this change or wait for a review of @TravisEz13 's edits?

@TylerLeonhardt
Copy link
Member Author

Thanks for the replies, everyone.

@Chuxel, we will need time to formulate the message from the release of this extension. Let us get back to you on the ownership. Note: I'm not signing up. We just need to discuss further.

Owning aside for a sec, I'd like to go back to the root of my concerns - the communication. I totally understand being heads down on something.

Like I said, we rely heavily on vscode and are pushing it into the space that PowerShell is prevalent in - IT and DevOps.

Lots of our customers have been very happy with the switch. Most if not all the talks at PS Summit NA last week used vscode.

Can I ask the vscode team to throw all the "language extension authors" on a DL and blast out communication about stuff like this Remote Development extension that way?

That way we can align with your message and make sure our (as in both your customers and ours - which are the same 😊) customers have the best experience.

* Update Dockerfile
@TylerLeonhardt
Copy link
Member Author

@TravisEz13 I've merged your PR into this one.

@Chuxel
Copy link
Member

Chuxel commented May 7, 2019

LGTM!

@Chuxel Chuxel merged commit 7ec3412 into microsoft:master May 7, 2019
@TylerLeonhardt TylerLeonhardt deleted the patch-1 branch May 7, 2019 15:59
@SteveL-MSFT
Copy link
Member

@Chuxel We should definitely collaborate and see what we can do to delight customers. The concern my team is sharing as well as myself is that before @TylerLeonhardt discovered this container, we didn't even know it existed. I don't think it should be expected that my team owns something we didn't know about nor planned for as we are equally busy with the commitments we've already made to our customers. Let's continue this conversation internally.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants