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

fix: remove the postfix License after the license name in AdditionalInfo #2573

Merged
merged 12 commits into from
Aug 16, 2024

Conversation

weyert
Copy link
Contributor

@weyert weyert commented Apr 25, 2024

Elements Default PR Template

In general, make sure you have: (check the boxes to acknowledge you've followed this template)

This pull request changes the way the license name gets rendered in HttpService and should fix #2523

@weyert weyert requested a review from a team as a code owner April 25, 2024 13:44
Copy link

netlify bot commented Apr 25, 2024

Deploy Preview for stoplight-elements ready!

Name Link
🔨 Latest commit a1edb55
🔍 Latest deploy log https://app.netlify.com/sites/stoplight-elements/deploys/66bf36c71179930008431398
😎 Deploy Preview https://deploy-preview-2573--stoplight-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Apr 25, 2024

Deploy Preview for stoplight-elements-demo ready!

Name Link
🔨 Latest commit a1edb55
🔍 Latest deploy log https://app.netlify.com/sites/stoplight-elements-demo/deploys/66bf36c7cef6100008683a2d
😎 Deploy Preview https://deploy-preview-2573--stoplight-elements-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@kentbulza
Copy link

I don't know the team philosophy when the data doesn't match the OpenAPI spec. You could not display at all if the strict terms "mutually exclusive" aren't followed ( https://spec.openapis.org/oas/v3.1.0#license-object ) ; or you could error. Probably should follow whatever's your convention.

There's not a test case for "name" alone. Only name is required and if name alone is there, then it seems to me it it should be just the name text. You don't want to get "undefined" as the link.

@weyert
Copy link
Contributor Author

weyert commented Apr 26, 2024

@kentbulza Thanks, I have updated it

@brendarearden
Copy link
Contributor

We do like this approach for fixing the postfix, but our team needs to do additional testing prior to merging this in.

Copy link
Contributor

@mnaumanali94 mnaumanali94 left a comment

Choose a reason for hiding this comment

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

LGTM!

@weyert
Copy link
Contributor Author

weyert commented Jul 26, 2024

Can we merge it? ❤️

@mnaumanali94
Copy link
Contributor

@weyert Can we fix the tests please and then we should be good to merge!

@weyert
Copy link
Contributor Author

weyert commented Aug 15, 2024

@mnaumanali94 I have updated the tests

@weyert
Copy link
Contributor Author

weyert commented Aug 15, 2024

The end to end tests are funning for me locally. Not sure what's up with the CI

       Spec                                              Tests  Passing  Failing  Pending  Skipped
  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
  │ ✔  api.cy.ts                                00:04       10       10        -        -        - │
  ├────────────────────────────────────────────────────────────────────────────────────────────────┤
  │ ✔  stoplight-project.cy.ts                  00:09       12       12        -        -        - │
  └────────────────────────────────────────────────────────────────────────────────────────────────┘
    ✔  All specs passed!                        00:14       22       22        -        -        -

@mnaumanali94 mnaumanali94 enabled auto-merge (squash) August 16, 2024 11:14
@mnaumanali94 mnaumanali94 merged commit e78e756 into stoplightio:main Aug 16, 2024
7 checks passed
@weyert weyert deleted the ticket- branch August 16, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$.info.license.name Always Add Word "License"
5 participants