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

Added 3.5 and 4.6.2 Samples #1

Merged
merged 4 commits into from
Nov 5, 2016
Merged

Added 3.5 and 4.6.2 Samples #1

merged 4 commits into from
Nov 5, 2016

Conversation

richlander
Copy link
Member

No description provided.

@richlander
Copy link
Member Author

richlander commented Nov 3, 2016

I'd like to see this better match the same scheme that I used for dotnet-docker-samples. We want the minimal directory structure and set of files. Here's my suggestion along those lines:

  • Drop the .sln (not really necessary) and the extra directory.
  • rename the csproj to match the dotnetapp-4.6.2 (for example) directory name.
  • I suspect you can drop the Properties/AssemblyInfo.cs
  • You need to keep the App.config.

Here's a simpler Environment block. I'd also suggest removing the ReadLine at the end. I added a using static System.Console as well.

            WriteLine("**Environment**");
            WriteLine($".NET Framework version: {(Environment.Version.Major == 4 ? "4.6.2" : "3.5") } ");
            WriteLine($"OS: {Environment.OSVersion}");

We can also remove the namespace from the sample.

@kendrahavens
Copy link
Contributor

  • Keeping .sln for ease of use per our discussion
  • Renamed csproj
  • When I drop the properties folder it will be regenerated anyway if the user opens the sample in VS. I'll keep it in for now.
  • Keeping App.config

<?xml version="1.0" encoding="utf-8"?>
<configuration>
<startup>
<supportedRuntime version="v4.0" sku=".NETFramework,Version=v4.6"/>

Choose a reason for hiding this comment

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

Update the TFM version to 4.6.2

@richlander
Copy link
Member Author

LGTM

@richlander richlander merged commit abcc9c7 into master Nov 5, 2016

if (args.Length > 0)
{
message = System.String.Join(" ", args);
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why System is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 907db09

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

Successfully merging this pull request may close these issues.

None yet

4 participants