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

Remove redundant project config, add file-scoped namespaces #1611

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

mattchenderson
Copy link
Contributor

@mattchenderson mattchenderson commented Nov 21, 2024

Cleaning up templates by removing some verbosity. Some of the settings in the project template have been unnecessary since the SDK provides many of these behaviors already. Of note, we're also removing the reference to Microsoft.Azure.Functions.Worker.Extensions.Http when Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore is included, since it's already pulling it in transitively. For the item templates, we're just moving to file-scoped namespaces (which involves removing indentation and is making the diff messy), as that gets rid of a small bit of clutter.

@mattchenderson mattchenderson force-pushed the mattchenderson/templatecleanup branch from 760e2cd to 1a87801 Compare November 21, 2024 22:09
@mattchenderson mattchenderson marked this pull request as ready for review November 21, 2024 22:16
<ItemGroup>
<Using Include="System.Threading.ExecutionContext" Alias="ExecutionContext"/>
</ItemGroup>
<!--#endif -->
<!--#if (NetFramework)-->
<ItemGroup>
<Folder Include="Properties\" />
Copy link
Contributor

Choose a reason for hiding this comment

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

this can also be removed.

namespace Company.Function
namespace Company.Function;

public class BlobTriggerCSharp
Copy link
Contributor

Choose a reason for hiding this comment

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

Could also use primary constructors.

Suggested change
public class BlobTriggerCSharp
public class BlobTriggerCSharp(ILogger<BlobTriggerCSharp> logger)

Then don't even have the _logger field. You can access the ctor parameter anywhere in this class via logger.

{
_logger = loggerFactory.CreateLogger<CosmosDBTriggerCSharp>();
}
public CosmosDBTriggerCSharp(ILoggerFactory loggerFactory)
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommend primary constructor and also switching to ILogger<CosmosDBTriggerCSharp>

{
using var blobStreamReader = new StreamReader(stream);
var content = await blobStreamReader.ReadToEndAsync();
_logger.LogInformation($"C# Blob Trigger (using Event Grid) processed blob\n Name: {name} \n Data: {content}");
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably not encourage bad logging practices in our samples (but not sure if that is in the scope of this PR)

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

Successfully merging this pull request may close these issues.

2 participants