Naming style for local functions #26165

Neme12 · 2018-04-15T19:28:58Z

I added a new option for local functions because it seems that people are divided on whether they should be named as locals or methods. For the same reason, I think we shouldn't give them a default style for now.

They're not even included in the default set of specifications. Users can to either include them with methods or add a new specification.

Neme12 · 2018-04-15T19:32:05Z

Tagging @jcouv @kuhlenh

CyrusNajmabadi · 2018-04-15T19:38:08Z

src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpNamingStyleDiagnosticAnalyzer.cs

+            SyntaxNodeAnalysisContext context,
+            ConcurrentDictionary<Guid, ConcurrentDictionary<string, string>> idToCachedResult)
+        {
+            var symbolContext = new SymbolAnalysisContext(


I'm pretty surprised this even had a public constructor you could use in this manner. @mavasani expected?

resolved in 3a806e7

CyrusNajmabadi · 2018-04-15T19:39:00Z

src/Features/Core/Portable/Diagnostics/Analyzers/NamingStyleDiagnosticAnalyzerBase.cs

+            OnCompilationStartAction(context, idToCachedResult);
+        }
+
+        protected virtual void OnCompilationStartAction(


can you make abstract, so all subclasses have to think about what the right thing to do is.

I usually prefer abstract methods but this is sort of like an event... you shouldn't think about it unless you need it. The local function thing is really a special case.

hrmm.. i'm on the fence.

fixed in e2356ba

CyrusNajmabadi · 2018-04-15T19:42:47Z

src/Workspaces/Core/Portable/NamingStyles/Serialization/NamingStylePreferences.cs

    internal class NamingStylePreferences : IEquatable<NamingStylePreferences>
    {
-        private const int s_serializationVersion = 4;
+        private const int s_serializationVersion = 5;


what impact does thsi have on people upgrading? will their existing preferences be honored?

I don't think so :/ this should reset your preferences

I'll take a look at what I can do to migrate them, but this is really hard to test

If we don't increase the version, the impact on newer versions would be that your settings for methods now apply to local functions as well.

ok. but you've confirmed that if you do update the version then previous settings are preserved and not wiped out?

Can we just write code that migrates across versions, and converts an existing method rule so that it only applies to ordinary methods, and not local functions? (we def need tests here as well).

the code in FromXElement a few lines below does wipe them out

the code in FromXElement a few lines below does wipe them out

Ok. we probably need to fix that. and have this code actually upgrade things from v4 to v5.

note: the upgrading codpath should run all the time, and it shoudl upgrade whenever the 'prior' value is before the change point.

i can't find any code in this PR related to ensuring user preferences are not wiped out. can you point me to it?

fixed in 8feada7

CyrusNajmabadi · 2018-04-15T19:44:01Z

src/Workspaces/Core/Portable/NamingStyles/Serialization/SymbolSpecification.cs

-                     : new XElement(nameof(TypeKind), TypeKind);
+                => SymbolKind.HasValue ? new XElement(nameof(SymbolKind), SymbolKind) :
+                   TypeKind.HasValue ? new XElement(nameof(TypeKind), TypeKind) :
+                   MethodKind.HasValue ? new XElement(nameof(MethodKind), MethodKind) :


what's the impact in the other direction. will previous versions of VS properly ignore this and not crash if you use this and have preferences-roaming on?

~~they should ignore it, the code in GetSymbolKindListFromXElement looks for names it knows, but I can't confirm... I think we need to upgrade the options~~

previous VS versions will notice that the version is different than what they like and they will reset the options

jcouv · 2018-04-15T21:12:00Z

@Neme12 Thanks! I'll look at the PR this week.

A few questions:

The screenshots you included show some specifications, but not naming specifications (casing and such). Is that included? (that was the primary purpose for Naming codestyles should handle local functions #22440)
Local functions can't have accessibility or modifiers. So the second screenshot seems strange to me. Does that dialog serve a purpose for local functions, or should we remove local functions from that dialog?
Do we provide defaults for naming other symbols? If so, should we provide defaults for local functions too? This is probably contentious and would need IDE team to decide, but it's probably better to do early in the lifecycle of the "local functions" features.

Neme12 · 2018-04-15T22:11:04Z

...Features/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs

-            await VerifyItemExistsAsync(markup, "MyClass", glyph: (int)Glyph.MethodPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
+            await VerifyItemExistsAsync(markup, "myClass", glyph: (int)Glyph.FieldPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
+            await VerifyItemExistsAsync(markup, "MyClass", glyph: (int)Glyph.PropertyPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);
+            await VerifyItemExistsAsync(markup, "GetMyClass", glyph: (int)Glyph.MethodPublic, expectedDescriptionOrNull: CSharpFeaturesResources.Suggested_name);


I have absolutely no idea why this changed... I'll try to investigate but for now I'm changing the test because the new behavior is better than the old one. MyClass now has a property rather than method glyph, which seems more appropriate. GetMyClass still has a method glyph as expected.

Before:

After:

ok with this change?

ok turns out this revealed an actual bug... your custom naming styles for methods now don't show up here... this is because a setting for MethodKind.Ordinary now doesn't match for a general SymbolKind.Method. This feature only tries to determine the possible SymbolKind of the declaration I'm typing, one of which is Method, but a code style setting for MethodKind.Ordinary doesn't match against any kind of Method. I have to fix this :/ and also add tests... The logic for determining which declaration I'm typing needs to be updated to recognize both methods & local functions and not lump them together in one SymbolKind.Method.

The fix involves fixing the name suggestion feature to respect settings for each individual method kind... for example if you have a certain suffix for methods but a different one for local functions, it should respect your style for each one depending on what you're naming and offer suggestions based on that.... this got a lot more complicated than I thought

Fixed in 8e556d2. I modified DeclarationNameCompletionProvider to differentiate between locals, local functions & methods. I did it in such a way that this icon change above is preserved by reordering the symbol checks to match properties first. I'll add more tests later.

I'm sorry but the test helpers look like a giant mess, many of the methods have 10 parameters, some of them are even virtual, and they use flags and null to determine what it's going to actually check in the first place. I don't know how to add anything to that,

You don't have to add parameters to existing methods. You can just make a new method. Something that asserts the order of the completion list.

and don't really feel like touching them at all.

Testing is part of coding. Just because an existing test system isn't in great shape doesn't mean that testing functionality can be skipped :)

the test is added as i said above

also this should have been tested before by the person who added the logic to customize the order

Sometimes we're not always lucky enough that the person before did all the things we wished :)

sharwell

I'd like to take a closer look at this. Marking request changes only do it doesn't get accidentally merged before I can check it out on Wednesday.

Neme12 · 2018-04-16T00:50:22Z

@jcouv

The screenshots you included show some specifications, but not naming specifications (casing and such). Is that included? (that was the primary purpose for #22440)

The sceenshots only show the list and definition of symbol specifications, which is only one part of naming style settings. The whole design consists of:

symbol specifications - you can choose which kind of symbols, accesibility and declaration modifiers each specification should match against. For example you can create a specification for "classes", "static fields" and another one for "async methods and local functions".
naming styles - you can create your own naming style consisting of a few options such as pascal/camelcase capitalization, word separator, prefix/suffix.
a list of naming rules - each rule binds one specification to one naming style. In this way, you can for example create a rule that says "async methods/local functions end with Async".

Note that I didn't change any of that. I didn't even change the default set of specifications to include one for local functions. I believe some people might want to include them together with their existing specifications for methods rather than adding a separate one. The only thing I added is the ability to choose "local function" as part of a symbol specification. You can create any kind of naming rule with this.

Local functions can't have accessibility or modifiers. So the second screenshot seems strange to me. Does that dialog serve a purpose for local functions, or should we remove local functions from that dialog?

Yes, but the list for symbols also contains parameters, which can't have any accesibility either. In fact the "modifiers" list will show irrelevant modifiers most of the time depending on what symbols you choose. We currently don't doo any filtering to narrow down the list of modifiers/accesibilities based on what set of symbols you have selected. I would prefer if I don't have to fix that.

Also note that they can have one of the modifiers - async. One other modifier I can think of is unsafe but that one isn't included as one of the options.

Do we provide defaults for naming other symbols? If so, should we provide defaults for local functions too? This is probably contentious and would need IDE team to decide, but it's probably better to do early in the lifecycle of the "local functions" features.

There are three rules by default.

Interfaces start with I.
Types (class, struct, interface, enum) are pascal case (I guess somebody didn't like delegates).
Non-field members (method, property, event) are pascal case.

There is a ton of default specifications however that you can see in the first screenshot.

CyrusNajmabadi · 2018-04-16T01:40:47Z

@Neme12 The PR is very heavy due to the rename of the SymbolAndTypeAndMethodKind. Can we undo that rename for now, and just get the primary changes in. Then, once it has passed review, the name change can be made.

Thanks!

CyrusNajmabadi · 2018-04-16T01:46:47Z

src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpNamingStyleDiagnosticAnalyzer.cs

+                isSupportedDiagnostic: _ => true,
+                context.CancellationToken);

+            SymbolAction(symbolContext, idToCachedResult);


instead of this approach, i woudl prefer if we registered a Symbol-op and a Syntax-op, and they just called through to the same final helper method, rather than synthesizing up a SymbolAnalysisContext instead and having the Syntax-op call through the Symbol-op version.

I don't know how. SymbolAction uses ReportDiagnostic from the context, Options and there is also an extension method on the context that it uses.

fixed in 3a806e7

CyrusNajmabadi · 2018-04-16T01:47:47Z

...rkspaces/Core/Portable/NamingStyles/EditorConfig/EditorConfigNamingStyleParser_SymbolSpec.cs

+        private static readonly SymbolOrTypeOrMethodKind _event = new SymbolOrTypeOrMethodKind(SymbolKind.Event);
+        private static readonly SymbolOrTypeOrMethodKind _delegate = new SymbolOrTypeOrMethodKind(TypeKind.Delegate);
+        private static readonly SymbolOrTypeOrMethodKind _parameter = new SymbolOrTypeOrMethodKind(SymbolKind.Parameter);
+        private static readonly ImmutableArray<SymbolOrTypeOrMethodKind> _all =


these bits of code are very hard to review since we have to go and validate each and every line. If you undo the rename it will make it much easier. Then the rename can happen at teh end once the overall logic has been validated.

fixed in 56df385

CyrusNajmabadi · 2018-04-16T01:50:15Z

src/Workspaces/Core/Portable/NamingStyles/Serialization/SymbolSpecification.cs

+                    new SymbolOrTypeOrMethodKind(MethodKind.LocalFunction),
+                    new SymbolOrTypeOrMethodKind(SymbolKind.Field),
+                    new SymbolOrTypeOrMethodKind(SymbolKind.Event),
+                    new SymbolOrTypeOrMethodKind(SymbolKind.Parameter)),


these lists are a PITA to review.

fixed in 56df385

Neme12 · 2018-04-16T06:40:52Z

@Neme12 The PR is very heavy due to the rename of the SymbolAndTypeAndMethodKind. Can we undo that rename for now, and just get the primary changes in. Then, once it has passed review, the name change can be made.

I can rename it back but unfortunately I don't think it's going to be easier to review because I updated almost every single usage of it.

CyrusNajmabadi · 2018-04-16T07:52:05Z

I can rename it back but unfortunately I don't think it's going to be easier to review because I updated almost every single usage of it.

There looked to be lots of cases where it was just a rename. so if we could change it back, it would be helpful. Thanks!

Neme12 · 2018-04-16T15:06:07Z

Question: @kuhlenh was asking for support for local variables... should I add them in as well? I think adding them on top of this would be easy with no extra changes required.

EDIT: well, there are many different places where you can declare a local in the language (and it's different for VB) so maybe it would be good for a separate PR after all... adding locals shouldn't require changing the serialization version so it's fine to do it separately.

jcouv · 2018-04-16T15:22:16Z

@Neme12 Thanks for the answer. Makes sense.
Do you know if naming conventions are controllable via editorconfig? Once this PR is merged and we adopt it, I'll want to set a preference for the Roslyn repo (or at least compiler portion) ;-)

Neme12 · 2018-04-16T15:30:28Z

@jcouv Yes! Here's an example:

If we later add support for locals as well, you could include them both in 1 specification if you want:

Neme12 · 2018-04-16T16:09:23Z

...Features/CSharp/Portable/Completion/CompletionProviders/DeclarationNameCompletionProvider.cs

+            foreach (var kind in declarationInfo.PossibleSymbolKinds)
            {
-                var kind = new SymbolKindOrTypeKind(symbolKind);
+                // There's no special glyph for local functions.


There probably should be. The Glyph enum has everything - ClassPublic, StructPublic, FieldPublic, FieldPrivate, Local, Parameter... local functions are clearly missing. They're just treated as private methods.

Can you open a bug?

filed #26327

DustinCampbell · 2018-04-16T17:18:51Z

Pinging @dotnet/roslyn-ide specifically. This should get some more eyes on it.

CyrusNajmabadi · 2018-04-16T19:00:13Z

...Features/CSharpTest/Completion/CompletionProviders/DeclarationNameCompletionProviderTests.cs

+            {
+                workspace.Options = workspace.Options.WithChangedOption(
+                    new OptionKey(SimplificationOptions.NamingPreferences, LanguageNames.CSharp),
+                    NamesEndWithSuffixPreferences());


do these tests run in parallel? if so, this would be a problem. Or is parallel running opt-in?

this is what the existing tests do

I thought there was a new test class instance created for each test, isn't that the case?

and yes, parallel running is disabled by default

Great. Thanks!

CyrusNajmabadi · 2018-04-16T19:01:36Z

src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpNamingStyleDiagnosticAnalyzer.cs

+            // HACK: RegisterSymbolAction doesn't work with local functions
+            context.RegisterSyntaxNodeAction(SyntaxNodeAction, SyntaxKind.LocalFunctionStatement);

+            void SyntaxNodeAction(SyntaxNodeAnalysisContext syntaxContext)


can you have an explicit "return" before this local function, then add a ``// local functions``` comment. it makes it much easier to tell that htese are locals, and not methods.

fixed in e2356ba

CyrusNajmabadi · 2018-04-16T19:02:16Z

src/Features/Core/Portable/Diagnostics/Analyzers/NamingStyleDiagnosticAnalyzerBase.cs

-            context.RegisterSymbolAction(c => SymbolAction(c, idToCachedResult), _symbolKinds);
+            context.RegisterSymbolAction(SymbolAction, _symbolKinds);
+
+            void SymbolAction(SymbolAnalysisContext symbolContext)


please place local functions after all normal method control flow. have an explicit 'return' before it, and add ``// local functions``` comment. thanks!

"// local functions" comment? :-/ that seems to me like "warning, new language features!"

Yes. Because local functions require extra attention over a normal method. Especially related to understanding capture flow. There is no obvious visual distinction between them and methods, so having the comment is very helpful. Otherwise, especially in github, you have to scroll all the way up to figure out if you are preceded by a statement and thus are a local function vs a sibling method.

Similarly, groupign all the local functions together, and not interspersing them in code, makes things much easier to analyze as you can look at the actual logic and code-flow in a single pass, without having random declarations interspersed that you have to jump past.

Like with all language constructs, there are ways to use them that can aide readability and comprehension and ways to use them that can hinder them. This is just like informal rules the IDE has about complexity in expressions and other things. You are not disallowed from using these features, but please use them in manners that are most conducive to a team environment.

fixed in e2356ba

CyrusNajmabadi · 2018-04-16T19:02:21Z

src/Features/Core/Portable/Diagnostics/Analyzers/NamingStyleDiagnosticAnalyzerBase.cs

+                    symbolContext.CancellationToken);
+
+                if (diagnostic != null)
+                    symbolContext.ReportDiagnostic(diagnostic);


fixed in e2356ba

CyrusNajmabadi · 2018-04-16T19:02:26Z

src/Features/CSharp/Portable/Diagnostics/Analyzers/CSharpNamingStyleDiagnosticAnalyzer.cs

+                    syntaxContext.CancellationToken);
+
+                if (diagnostic != null)
+                    syntaxContext.ReportDiagnostic(diagnostic);


fixed in e2356ba

…ngStyle

Neme12 · 2018-05-28T14:46:19Z

@sharwell What would be the best design to achieve that? I got a few ideas:

When creating the option in SimplificationOptions.NamingPreferences:

roslyn/src/Workspaces/Core/Portable/Simplification/SimplificationOptions.cs

Lines 103 to 106 in d6f483e

    
           internal static PerLanguageOption<NamingStylePreferences> NamingPreferences { get; } = new PerLanguageOption<NamingStylePreferences>(nameof(SimplificationOptions), nameof(NamingPreferences), defaultValue: NamingStylePreferences.Default, 
        
               storageLocations: new OptionStorageLocation[]{ 
        
                   new NamingStylePreferenceEditorConfigStorageLocation(), 
        
                   new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences")});

add one more RoamingProfileStorageLocation to the storageLocations array.

            storageLocations: new OptionStorageLocation[]{
                new NamingStylePreferenceEditorConfigStorageLocation(),
                new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences2"),
                new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences")});

RoamingVisualStudioProfileOptionPersister would then have to be modified to expect potentially more than 1 RoamingProfileStorageLocation (in the order of priority) and when fetching the value, try all of them in order until a value is found. When storing a value, it would always use the first one.

Instead of adding more than 1 RoamingProfileStorageLocation, modify the RoamingProfileStorageLocation class to support versioning (maybe a list of suffixes in priority order, or simply the number of versions)?
Do not modify RoamingProfileStorageLocation but instead create a new class called VersionedRoamingProfileStorageLocation which would support this functionality.

sharwell · 2018-05-28T15:17:01Z

@Neme12 Did not think of option 1 at all, but if it works and isn't an overwhelming about of code I love it!

Neme12 · 2018-05-28T15:42:53Z

...Portable/Completion/CompletionProviders/DeclarationNameCompletionProvider.DeclarationInfo.cs

                        GetNodeDenotingTheTypeOfTupleArgument,
                        _ => default(SyntaxTokenList),
-                        _ => ImmutableArray.Create(SymbolKind.Local), cancellationToken);
+                        _ => ImmutableArray.Create(new SymbolKindOrTypeKind(SymbolKind.Local)), cancellationToken);


This is technically wrong. User's preferences for locals shouldn't apply to tuple name suggestions. This needs to be fixed as soon as naming preferences for tuples are implemented.

Neme12 · 2018-05-28T16:18:52Z

@sharwell I'm trying to manually test the behavior with roaming settings and the parallel settings for different versions work correctly, but I don't know how to test the upgrade path since that would require resetting roaming settings that are associated with my account. Do you know if there's a way to do that?

sharwell · 2018-05-28T16:56:28Z

I'm trying to manually test the behavior with roaming settings and the parallel settings for different versions work correctly, but I don't know how to test the upgrade path since that would require resetting roaming settings that are associated with my account. Do you know if there's a way to do that?

I'm not sure. @olegtk are you aware of a test strategy for this?

…format into an older version

Neme12 · 2018-05-28T17:43:33Z

src/VisualStudio/Core/Def/Implementation/Options/RoamingVisualStudioProfileOptionPersister.cs

+            {
+                var storageKey = roamingSerialization.GetKeyNameForLanguage(optionKey.Language);
+
+                RecordObservedValueToWatchForChanges(optionKey, storageKey);


I'm not completely certain whether this is the right thing to do. Should only ever listen to changes in the newest location? Or in the one that we found?

This seems the most correct - any change to the location we found or any previous ones will affect the result. I'm just not sure whether this will work properly.

Actually, I don't see any problem with this after looking at the implementation. But I don't know how to verify the behavior.

I see why this seemed confusing, but I agree with the conclusion.

Neme12 · 2018-05-28T17:47:30Z

src/Workspaces/Core/Portable/Simplification/SimplificationOptions.cs

+            storageLocations: new OptionStorageLocation[] {
                new NamingStylePreferenceEditorConfigStorageLocation(),
-                new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences")});
+                new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences[5]"),


I used 5 instead of 2 after NamingStylePreferences.s_serializationVersion, which is 5.

❔ Are we sure brackets are allowed? I was expecting to see NamingPreferences5.

Neme12 · 2018-05-28T17:55:12Z

@sharwell I was able to verify this manually by changing the location name (so that it doesn't exist in my roaming settings). But I'm still a little worried about the fact that there are no unit tests for RoamingVisualStudioProfileOptionPersister.

sharwell · 2018-05-28T17:59:34Z

src/Workspaces/Core/Portable/Simplification/SimplificationOptions.cs

+            storageLocations: new OptionStorageLocation[] {
                new NamingStylePreferenceEditorConfigStorageLocation(),
-                new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences")});
+                new RoamingProfileStorageLocation("TextEditor.%LANGUAGE%.Specific.NamingPreferences[5]"),


❔ Are we sure brackets are allowed? I was expecting to see NamingPreferences5.

Neme12 · 2018-05-28T18:09:07Z

❔ Are we sure brackets are allowed? I was expecting to see NamingPreferences5.

I verified this after the change to brackets and it worked correctly. I thought it might be nice to use a special convention for different versions of the same value, to clearly differentiate this pattern. But I don't feel strongly about this, please let me know you would prefer to change it back.

Neme12 · 2018-05-29T20:23:53Z

@sharwell I made a minor change to fix an outdated comment I noticed while reviewing the code and added a proper test case now that naming for local variables has been merged.

Neme12 · 2018-05-29T20:36:12Z

retest this please

Neme12 · 2018-05-29T20:37:44Z

is CI stuck?

sharwell · 2018-05-29T21:04:01Z

@Neme12 Not sure. Will get back to you on the naming later today. We'll deal with the CI when we're trying to get it merged. 😄

Neme12 · 2018-05-31T17:57:37Z

Will get back to you on the naming later today

@sharwell any new info on that?

Also, will this PR target 15.8 or not (since the changes in VS persistence might be risky)?

sharwell · 2018-05-31T18:41:26Z

@Neme12 We reached a weak consensus on NamingPreferences5. If you switched to the form without the brackets I don't think anyone would object.

I am planning this for 15.8 along with the change to the meaning of empty/*.

sharwell · 2018-05-31T19:18:27Z

@jinujoseph for approval

jinujoseph · 2018-05-31T19:28:12Z

Approved to merge for 15.8.Preview3
@kuhlenh as info on the UI changes here

Neme12 · 2018-06-01T07:18:25Z

retest windows_debug_spanish_unit32_prtest please

Neme12 requested a review from a team as a code owner April 15, 2018 19:28

CyrusNajmabadi reviewed Apr 15, 2018

View reviewed changes

jcouv added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Apr 15, 2018

jcouv added this to the 15.8 milestone Apr 15, 2018

Neme12 commented Apr 15, 2018

View reviewed changes

sharwell suggested changes Apr 15, 2018

View reviewed changes

CyrusNajmabadi reviewed Apr 16, 2018

View reviewed changes

Neme12 force-pushed the localFunctionNamingStyle branch from 6362d1d to 56df385 Compare April 16, 2018 12:40

Neme12 commented Apr 16, 2018

View reviewed changes

CyrusNajmabadi reviewed Apr 16, 2018

View reviewed changes

Neme12 added 2 commits May 28, 2018 13:50

Merge remote-tracking branch 'upstream/master' into localFunctionNami…

990246e

…ngStyle

Fix after merge

3f00166

Neme12 commented May 28, 2018

View reviewed changes

Adding new roaming location to prevent syncing the new serialization …

d9262c2

…format into an older version

Neme12 commented May 28, 2018

View reviewed changes

sharwell approved these changes May 28, 2018

View reviewed changes

Removing outdated comment & adding missing test case

352226e

Switching to form without brackets

ed4f34c

jinujoseph added the Approved to merge label May 31, 2018

sharwell merged commit e3dcf8c into dotnet:master Jun 1, 2018

Neme12 deleted the localFunctionNamingStyle branch June 1, 2018 13:50

Neme12 mentioned this pull request Jun 1, 2018

Expand persister deserialization to also take into account string and accessibility modifiers #27193

Merged

Therzok mentioned this pull request Jun 26, 2018

Roaming profile serialization behaviour has changed in roslyn mono/monodevelop#5208

Closed

jmarolf mentioned this pull request Jul 6, 2018

Updating the OptimizationData to 2.9.0-beta7-63018-03-01 #28332

Merged

Naming style for local functions #26165

Naming style for local functions #26165

Uh oh!

Conversation

Neme12 commented Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Neme12 commented Apr 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv commented Apr 15, 2018

Uh oh!

Neme12 Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Neme12 Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Neme12 commented Apr 15, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 15, 2018 •

edited

Loading

Neme12 Apr 15, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 15, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 15, 2018 •

edited

Loading

Neme12 Apr 15, 2018 •

edited

Loading

Neme12 Apr 15, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 17, 2018 •

edited

Loading

Neme12 commented Apr 16, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading

Neme12 Apr 16, 2018 •

edited

Loading