Skip to content

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented Dec 7, 2021

A warnlevel 7 warning is being added in VS 17.1 / SDK 6.0.200 to warn about lower-cased ascii type names. (See PR).

@jcouv jcouv self-assigned this Dec 7, 2021
@ghost
Copy link

ghost commented Dec 7, 2021

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@jaredpar
Copy link
Member

jaredpar commented Dec 7, 2021

Interesting. Most seem to be testing types in the DLR / CSharp dynamic binder code which effectively we own. Looks like a handful of other types hit this but it is more common than I would've expected.

@danmoseley
Copy link
Member

Can we just fix the casing? I assume they're all internal and we wouldn't write new code this way. Unless you want to dog food your warning.

@jcouv
Copy link
Member Author

jcouv commented Dec 8, 2021

Can we just fix the casing?

That's an option too, but is a bit more work. So you tell me what you'd like ;-)
How about upper-casing the ones in the product code, but leaving the tests merely escaped?

@jcouv jcouv requested a review from stephentoub December 8, 2021 01:13
@danmoseley
Copy link
Member

I'd prefer to upper case them all, but if that's a hassle this change is OK to me.

@jcouv
Copy link
Member Author

jcouv commented Dec 8, 2021

Thanks for letting me know. I'll take a stab.

@jcouv
Copy link
Member Author

jcouv commented Dec 8, 2021

How should rtmsg, nlmsgerr and nlmsghdr be capitalized?

@jcouv jcouv marked this pull request as ready for review December 8, 2021 08:33
@ghost
Copy link

ghost commented Dec 8, 2021

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

A warnlevel 7 warning is being added in VS 17.1 / SDK 6.0.200 to warn about lower-cased ascii type names. (See PR).

Author: jcouv
Assignees: jcouv
Labels:

area-Meta

Milestone: -

@stephentoub
Copy link
Member

How should rtmsg, nlmsgerr and nlmsghdr be capitalized?

I'd prefer to leave these lower-cased. We have a policy that types/functions declared in C# for interop should mirror the native naming, as if we were able to simply reference a .h file. On Unix, that invariably means lower-cased names.

@jcouv
Copy link
Member Author

jcouv commented Dec 8, 2021

Then hostent, cgroups, procfs should be left lower-cased too?

This reverts commit 1b31d9f.
@stephentoub
Copy link
Member

Then hostent, cgroups, procfs should be left lower-cased too?

Similarly my preference is for those to remain lower-cased as well.

@jcouv jcouv requested a review from danmoseley December 9, 2021 00:45
Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

Thanks

@jcouv jcouv merged commit ab0de53 into dotnet:main Dec 9, 2021
@jcouv jcouv deleted the lower-case branch December 9, 2021 08:05
@gbalykov gbalykov mentioned this pull request Dec 9, 2021
@kasperk81
Copy link
Contributor

merged with libraries builds failing, affecting everyone else

@adamsitnik adamsitnik mentioned this pull request Dec 9, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jan 8, 2022
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.

6 participants