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

Added first version of Ogooreck Api implementation #1

Merged
merged 5 commits into from
Jun 15, 2022
Merged

Conversation

oskardudycz
Copy link
Owner

No description provided.

@oskardudycz oskardudycz force-pushed the Ogooreck_Api branch 2 times, most recently from b61d2f6 to e3b10b6 Compare April 16, 2022 15:32
@oskardudycz oskardudycz changed the title Added first compiled version of Ogooreck Api implementation Added first version of Ogooreck Api implementation Apr 16, 2022
README.md Outdated
@@ -1,2 +1,36 @@
# Ogooreck

Sneaky Test library

Choose a reason for hiding this comment

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

More description here about what the library does would be useful

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added more in recent commit 🙂

<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
Copy link

@TheMagnificent11 TheMagnificent11 Apr 16, 2022

Choose a reason for hiding this comment

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

As this is going to be a public library, I would suggest something like this...

  <PropertyGroup>
    <TargetFramework>net6.0</TargetFramework>
    <PackageProjectUrl>
      https://github.com/oskardudycz/Ogooreck
    </PackageProjectUrl>
    <RepositoryUrl>
      https://github.com/oskardudycz/Ogooreck.git
    </RepositoryUrl>
    <PublishRepositoryUrl>true</PublishRepositoryUrl>
    <Product>Ogooreck</Product>
    <GenerateDocumentationFile>true</GenerateDocumentationFile
    <EmbedUntrackedSources>true</EmbedUntrackedSources
    <AllowedOutputExtensionsInPackageBuildOutputFolder>
      $(AllowedOutputExtensionsInPackageBuildOutputFolder);.pdb
    </AllowedOutputExtensionsInPackageBuildOutputFolder>
    <IncludeSymbols>true</IncludeSymbols>
    <SymbolPackageFormat>snupkg</SymbolPackageFormat
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <WarningsAsErrors />
    <LangVersion>10.0</LangVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="FluentAssertions" Version="6.6.0"/>
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Testing" Version="6.0.4"/>
    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1">
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added in: 082a3d0.


namespace Ogooreck.API;

public static class ApiSpecification

Choose a reason for hiding this comment

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

The class has no documentation comments, which would extremely useful for new users of your library

Copy link
Owner Author

Choose a reason for hiding this comment

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

This will come in a separate PR. I created a dedicated issue for it: #2.

///////////////////
//// GIVEN ////
///////////////////
public static Func<HttpRequestMessage, HttpRequestMessage> URI(string uri) =>

Choose a reason for hiding this comment

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

The correct camel-casing would be Uri, wouldn't it?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Same comment as #1 (comment).

public static Action<HttpResponseMessage> PRECONDITION_FAILED = HTTP_STATUS(HttpStatusCode.PreconditionFailed);
public static Action<HttpResponseMessage> METHOD_NOT_ALLOWED = HTTP_STATUS(HttpStatusCode.MethodNotAllowed);

public static Action<HttpResponseMessage> HTTP_STATUS(HttpStatusCode status) =>
Copy link

@TheMagnificent11 TheMagnificent11 Apr 16, 2022

Choose a reason for hiding this comment

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

Personally, I'm not a fan of the "all caps" naming that you've got throughout this file...would prefer camel-casing, which I believe is the Microsoft code-style standard for .Net

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for the suggestion. I'll keep that in mind, and gather also other feedback around that. I like that it makes tests more explicit and the specific parts highlighted, so I'll keep it as it is for now.

response => response.StatusCode.Should().Be(status);
}

public class ApiSpecification<TProgram>: IDisposable where TProgram : class
Copy link

@TheMagnificent11 TheMagnificent11 Apr 16, 2022

Choose a reason for hiding this comment

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

Separates file for separate classes?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think that for now it's simple enough to keep it in the same file, but in the long term, I agree, it'd be good to keep it separated when it starts to grow.

- final version of the first prerelease nuget
- docs
- github actions
@oskardudycz oskardudycz marked this pull request as ready for review June 15, 2022 16:48
@oskardudycz oskardudycz merged commit 89b3b5d into main Jun 15, 2022
@oskardudycz oskardudycz deleted the Ogooreck_Api branch June 15, 2022 16:48
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