Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

AspnetCore 2.0 sample #2878

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bakerbrian
Copy link

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the Nancy code style guidelines
  • I have provided test coverage for my change (where applicable)

Description

Added AspnetCore 2.0 hosted sample

@dnfclas
Copy link

dnfclas commented Mar 18, 2018

CLA assistant check
All CLA requirements met.

@@ -0,0 +1,15 @@
using Microsoft.AspNetCore;
Copy link
Member

Choose a reason for hiding this comment

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

Can we move the using statements inside the namespace, to align with the rest of the Nancy code

@@ -0,0 +1,31 @@
using Microsoft.AspNetCore.Builder;
Copy link
Member

Choose a reason for hiding this comment

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

Can we move the using statements inside the namespace, to align with the rest of the Nancy code

app.UseOwin().UseNancy();

app.Run(async (context) =>
{
Copy link
Member

Choose a reason for hiding this comment

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

The indentation is a bit off here

Copy link
Member

Choose a reason for hiding this comment

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

Why even include this block of code? The Nancy middleware should handle the request, so this should never be hit.

app.Run(async (context) =>
{
await context.Response.WriteAsync("Hello World!");
});
Copy link
Member

Choose a reason for hiding this comment

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

The indentation is a bit off here

public void Configure(IApplicationBuilder app, IHostingEnvironment env)
{
if (env.IsDevelopment())
app.UseDeveloperExceptionPage();
Copy link
Member

Choose a reason for hiding this comment

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

We need to wrap this line in curly-braces so align with the rest of the code

Pull request amendments.
@bakerbrian
Copy link
Author

@thecodejunkie, @khellang, thank you for your feedback, the amendments have been committed as per your comments.

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.

4 participants