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 author , owner and description to NuGet generation #160

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tagz97
Copy link

@tagz97 tagz97 commented Jan 11, 2023

Why

I have added these changes to support adding of a custom description, author and owner for packing NuGet packages and symbols to give more flexibility and control to the user. I have also added a small convenient error message when the method name is null (OperationId is missing/null) as this initially cost me several hours of trying to figure out what may or may not have been wrong with my own OpenAPI spec.

Changes

  • Added author to the Generate Options and Yardarm Generation Settings
  • Added description to the Generate Options and Yardarm Generation Settings
  • Added owner to the Generate Options and Yardarm Generation Settings
  • Assign author, description and owner from Generation Options to Yardarm Generation Settings
  • Modify logic on selecting description for NuGet packaging by introducing Description but allowing to use AssemlyName if null and introducing Owner
  • Added a friendly error message to TagTypeGeneratorBase to assist a user in finding an error with their OpenAPI spec and prevent an unhandled exception from being thrown due to a null value
  • Updated the generation README.md to include an example of providing an author, owner and a description as part of generating a NuGet using Yardarm.CommandLine

Result

NuGet and symbols packages are generated with a custom Author and Custom description. This description does not overwrite the description within the Spec.
image

Passing Tests

image

- Added author to the Generate Options and Yardarm Generation Settings
- Added description to the Generate Options and Yardarm Generation Settings
- Assign author and description from Generation Options to Yardarm Generation Settings
- Modify logic on selecting description for NuGet packaging by introducing Description but allowing to use AssemlyName if null
- Added a friendly error message to TagTypeGeneratorBase to assist a user finding an error with their OpenAPI spec and prevents and unhandled exception from being thrown due to a null value
- Updated the generation README.md to include an example of providing author and description as part of generating a NuGet using Yardarm.CommandLine
@tagz97 tagz97 marked this pull request as ready for review January 11, 2023 13:43
@tagz97 tagz97 closed this Jan 11, 2023
@tagz97
Copy link
Author

tagz97 commented Jan 11, 2023

Apologies I closed it by mistake, I have reopened it

@tagz97 tagz97 reopened this Jan 11, 2023
@tagz97 tagz97 changed the title Added author and description to NuGet generation Added author , owner and description to NuGet generation Jan 12, 2023
@brantburnett brantburnett self-assigned this Feb 14, 2023
Copy link
Contributor

@brantburnett brantburnett left a comment

Choose a reason for hiding this comment

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

I apologize that it's taken me so long to get back to you on this, it got lost in the shuffle. It looks great overall, and thanks for the PR, I've just added a couple of minor comments.

@@ -29,6 +29,8 @@ public class YardarmGenerationSettings
public Version Version { get; set; } = new Version(1, 0, 0);
public string? VersionSuffix { get; set; }
public string Author { get; set; } = "anonymous";
public string Owner { get; set; } = "anonymous";
public string Description { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

This one needs a nullable annotation.

Suggested change
public string Description { get; set; }
public string? Description { get; set; }

Comment on lines +33 to +39
// This is here to show a useful error that there is an error with the Spec provided. If the operation
// is missing the name attribute in the OpenAPI spec, the MethodDeclaration step will fail
if (string.IsNullOrEmpty(methodName))
{
throw new NullReferenceException($"{nameof(GenerateOperationMethodHeader)} ran into an error. Please ensure the method at the path {operation.Key} {operation} is decorated correctly with the OperationId present.");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

There's actually a separate issue already open to address this problem, I'd prefer to leave it off this PR for the sake of the separation of issues.

Suggested change
// This is here to show a useful error that there is an error with the Spec provided. If the operation
// is missing the name attribute in the OpenAPI spec, the MethodDeclaration step will fail
if (string.IsNullOrEmpty(methodName))
{
throw new NullReferenceException($"{nameof(GenerateOperationMethodHeader)} ran into an error. Please ensure the method at the path {operation.Key} {operation} is decorated correctly with the OperationId present.");
}

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

Successfully merging this pull request may close these issues.

2 participants