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

Description & summary of Operations not showing in changelog. #60

Closed
msnijder30 opened this issue Oct 19, 2018 · 20 comments
Closed

Description & summary of Operations not showing in changelog. #60

msnijder30 opened this issue Oct 19, 2018 · 20 comments

Comments

@msnijder30
Copy link
Contributor

msnijder30 commented Oct 19, 2018

Description

I've been testing what this library can and cannot do, and it appears that it does not keep track of description & summary of the operations?

I have also tested this with the description of parameters, and they do update.

It does show the summary directly after the path of the operation, but it only shows the latest one and it does not show it has been changed.

The code

  /pets/{id}:
    get:
      description: A description    // This is not in changelog when changed
      summary: This is a summary    // This is not in changelog when changed
      operationId: find pet by id
      parameters:
        - name: aaadda
          in: path
          description: Param desc   // This is in changelog when changed
          required: true
          schema:
            type: integer
            format: int64

Question

Am I correct in that those properties are not in the Changelog? Or am I making a mistake.

@quen2404
Copy link
Member

What kind of changelog ? Html / Cli generation are not up to date, but markdown is.
The library keep track of changes in description & summary, but this is not reported in the changelog currently. I didn't show this kind of changes in the changelog because it's only descriptive data, not change in the API design.

@msnijder30
Copy link
Contributor Author

I was using HtmlRender, I will do a test using MarkdownRender.

@msnijder30
Copy link
Contributor Author

description is not showing up in MarkdownRender either, I think it would be nice to show those changes aswell, if possible

@quen2404
Copy link
Member

Ok, in my company we only use the Markdown output. I will update HtmlRender to output the same details.
But as i mentionned in my previous response, this kind of changes are only descriptive.
I understand you want to see all changes in the changelog :) So as a compromise, we can add an option to output this kind of descriptive changes in the changelog !

@msnijder30
Copy link
Contributor Author

msnijder30 commented Oct 19, 2018

That would be a great feature I think! Is something like that a lot of work to implement?

@quen2404
Copy link
Member

It depends on what is your definition of quick 😉 I'm a little busy today and this weekend, but i think i will have time to do this next week.

@msnijder30
Copy link
Contributor Author

msnijder30 commented Oct 19, 2018

That's pretty quick, thanks for taking the time to create this feature!

To clarify, I was using the Direct Invocation method using the dependency on Maven. Not the CLI 😊

msnijder30 added a commit to msnijder30/openapi-diff that referenced this issue Oct 19, 2018
@msnijder30
Copy link
Contributor Author

msnijder30 commented Oct 19, 2018

Hi, I had some time, looked at the code and made something.

What's new

I've added the classes OperationMetadataDiff and ChangedOperationMetadata. These classes keep track of changes made in summary & description.

I've also added the setting showChangedMetadata to MarkdownRenderer, which is false by default, this is the option which will enable descriptive changes.

The changed markdown

As you can see there is an Metadata field as first item below the operation. This contains the summary & description. If only the summary has changed it will only show the summary, same for the description.

If showChangedMetadata is enabled and if there are changes the markdown file looks as follows:

#### What's Changed
---

##### `GET` /pets/{id}

> This is a summary

###### Metadata:

Changed summary : `Old summary` to: 
> `This is a summary`

Changed description : `Returns a user if the user does not have access to the pet` to: 
> `A description`

###### Parameters:

Added: `aaadda` in `path`
> Param desc

Deleted: `id` in `path`
> ID of pet to fetch

Example code

MarkdownRender renderer = new MarkdownRender();
renderer.setShowChangedMetadata(true);

String md = renderer.render(diff);

All commits related to this feature

  • cd827cf (Changed OpenApiDiff to have more Lombok)
  • be1c577 (First working version)
  • 1da0e19 (Final working version)

Questions

  • What do you think of this?
  • What do you think of the name metadata to describe the summary & description?
  • Is the formatting of the markdown acceptable?
  • In ChangedOperationMetadata#isChanged I'm not sure if there also should be an INCOMPATIBLE option since it is compatible with OpenAPI 2.0 but not with OpenAPI 1.2, in 1.2 there is no description field. It should be backwards compatible though since it will just see that the description field is null and will not include it into the changelog. summary should still work.

Looking forward to your reaction, if you think this is a good idea I'll make a pull request, if you have any suggestion for changes I'll add those.

If you think it's a bad idea that's okay aswell 👍

@quen2404
Copy link
Member

Thanks. I'll review this shortly

@msnijder30
Copy link
Contributor Author

Any updates on the issue?

@quen2404
Copy link
Member

Hello, i opened the project this morning !
I'm working on it !

@quen2404
Copy link
Member

I'm working on it in the branch feature/summary-description, but work is in progress.

@quen2404
Copy link
Member

I prefer work on a dedicated branch cause your code was only covering summary and description in Operation. But i'm open to any suggestion 😉

@msnijder30
Copy link
Contributor Author

That sounds good, I was interested in those items mostly but it seems only logical to do it for more items than just Operation indeed 👍

@quen2404
Copy link
Member

quen2404 commented Nov 5, 2018

Fixed with pull request #61 ! 😄

@quen2404 quen2404 closed this as completed Nov 5, 2018
@quen2404
Copy link
Member

quen2404 commented Nov 5, 2018

@msnijder30 i only add support in diff result, not on the ouput (markdown, html, console). Because i'm working on another branch to improve the output globally (not only for summary/description)

@msnijder30
Copy link
Contributor Author

Okay, perfect! Thanks a lot for this feature.
I was also wondering if the latest snapshot build was available through maven but I was not able to find it on the snapshot repository, am I overlooking something?

@quen2404
Copy link
Member

quen2404 commented Nov 5, 2018

Snapshot version is not available currently 😐

@msnijder30
Copy link
Contributor Author

Any idea when the next snapshot/official version release will be?

@quen2404
Copy link
Member

quen2404 commented Nov 5, 2018

I don't know, i have some idea to improve code, and it will break the current api. So I want to make this changes before release 2.0.0 ! Not big changes, but enough to break compatibility !

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

No branches or pull requests

2 participants