Skip to content
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

Setup some infrastructure for the repository. #60

Merged
merged 7 commits into from Jun 1, 2019

Conversation

Projects
None yet
2 participants
@tannergooding
Copy link
Member

commented May 31, 2019

This sets up a lot of infrastructure for the repository including using Directory.Build.props/targets and adding various scripts for building, restoring, testing, and packing.

I believe the ultimate end-goal is to get on dotnet/arcade, but this should still be a step in the right direction.

It does not currently enable strong-name signing (as it needs to be determined if a custom or existing strong name key will be used). It would also be trivial to hook up authenticode signing once that is resolved.

If we have an existing AzDO repo, I can also add the azure-pipelines.yml file and get that enabled.

# suggest static readonly declarations be pascal case
# suggest type parameters be prefixed with T
###############################################################################
[*.cs]

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

These all match the VS defaults modulo csharp_indent_case_contents_when_block

csharp_indent_block_contents = true
csharp_indent_braces = false
csharp_indent_case_contents = true
csharp_indent_case_contents_when_block = false

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

This is the only new option that differs from the VS default. The VS default is true for compatibility reasons. This option is used in combination with csharp_indent_case_contents and you have effectively the following combinations:

(VS Default) csharp_indent_case_contents=true, csharp_indent_case_contents_when_block=true:

switch (value)
{
    case 1:
        break;

    case 2:
        {
            break;
        }
}

csharp_indent_case_contents=false, csharp_indent_case_contents_when_block=true:

switch (value)
{
    case 1:
    break;

    case 2:
        {
            break;
        }
}

csharp_indent_case_contents=false, csharp_indent_case_contents_when_block=false:

switch (value)
{
    case 1:
    break;

    case 2:
    {
        break;
    }
}

(The one I chose) csharp_indent_case_contents=true, csharp_indent_case_contents_when_block=false:

switch (value)
{
    case 1:
        break;

    case 2:
    {
        break;
    }
}

This comment has been minimized.

Copy link
@mjsabby

mjsabby May 31, 2019

Member

I personally don't like the corefx style, but since you've written most of the code here now you can pick whatever you want.

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

Which one of the four is your preference? I don't feel particularly strongly as long as the editorconfig ensures it is automatically normalized.

This comment has been minimized.

Copy link
@mjsabby

mjsabby Jun 1, 2019

Member

The last one seems ok. I don't know why it's not the default. The VS Default looks ugly.

This comment has been minimized.

Copy link
@tannergooding

tannergooding Jun 1, 2019

Author Member

The last one is the one I already choose for the repo (and was the one I thought you were indicating you didn't like) 😄

The first is the VS default for compatibility reasons at this point. The option allowing the last setup was only introduced a couple versions ago (I think VS2015 or 2017) and hasn't been around for very long at this point.

*.suo
*.user
*.sln.docstates
###############################################################################

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

The main .gitignore file has a lot more than necessary. I can keep it if desired, but I personally find the smaller subset easier to reason about.

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

(It also helps expose places where things aren't ending up where expected as almost all excluded files should end up under artifacts).

This comment has been minimized.

Copy link
@mjsabby

mjsabby May 31, 2019

Member

I agree ... I just let GitHub decide so this is better imo too.

@@ -0,0 +1,73 @@
<?xml version="1.0" encoding="utf-8"?>
<Project>

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

Centralized basically all of the relevant project settings here.

Show resolved Hide resolved Directory.Build.props Outdated
</ItemGroup>

<!-- Package versions for package references across all projects -->
<ItemGroup>

This comment has been minimized.

Copy link
@tannergooding

tannergooding May 31, 2019

Author Member

Package versions are centrally managed here.

This was referenced May 31, 2019

@mjsabby

This comment has been minimized.

Copy link
Member

commented May 31, 2019

What do you mean by existing AzDO repo? Do we need a repo? I thought we just need an instance.

@tannergooding

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

Right, I meant instance. Something like you have for the LLVMSharp repo.

@mjsabby

This comment has been minimized.

Copy link
Member

commented May 31, 2019

Ok ... I'd love if it could be in dnceng .

@tannergooding

This comment has been minimized.

Copy link
Member Author

commented May 31, 2019

Ok ... I'd love if it could be in dnceng

I've pinged the e-mail thread to see what is recommended here.

@mjsabby

mjsabby approved these changes Jun 1, 2019

@tannergooding tannergooding merged commit 23e8db8 into microsoft:master Jun 1, 2019

1 check passed

license/cla All CLA requirements met.
Details

@tannergooding tannergooding deleted the tannergooding:infrastructure branch Jun 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.