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

Update views versions #1857

Open
wants to merge 5 commits into
base: 6.13.x
Choose a base branch
from
Open

Conversation

altro3
Copy link
Collaborator

@altro3 altro3 commented Nov 8, 2024

OpenAPI Explorer 2.2.732
Rapidoc 9.3.8
ReDoc 2.2.0
Swagger UI 5.18.2

Rapidoc 9.3.8
ReDoc 2.2.0
Swagger UI 5.18.2
Copy link
Contributor

@sdelamo sdelamo left a comment

Choose a reason for hiding this comment

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

@altro3 can you please add links to where these files are being copied from? Also which licenses these files have.

It is hard to review a PR with minimized javascript files.

@altro3
Copy link
Collaborator Author

altro3 commented Nov 8, 2024

OpenApiExplorer: https://unpkg.com/[email protected]/dist/browser/openapi-explorer.min.js
Sources repo: https://github.com/Authress-Engineering/openapi-explorer

Rapidoc: https://unpkg.com/[email protected]/dist/rapidoc-min.js
Sources repo: https://github.com/rapi-doc/RapiDoc

Redoc: https://unpkg.com/[email protected]/bundles/redoc.standalone.js
Sources repo: https://github.com/Redocly/ReDoc/

Swagger UI: https://unpkg.com/[email protected]/dist/swagger-ui.js
Sources repo: https://github.com/swagger-api/swagger-ui

Where I need to add licenses?

And why we need add licenses only for these libs? Then we should add licenses for each of the libraries we use. But I don't think it makes sense, because micronaut is not a commercial product.

@altro3 altro3 requested a review from sdelamo November 8, 2024 06:55
@sdelamo
Copy link
Contributor

sdelamo commented Nov 8, 2024

OpenApiExplorer: https://unpkg.com/[email protected]/dist/browser/openapi-explorer.min.js Sources repo: https://github.com/Authress-Engineering/openapi-explorer

Rapidoc: https://unpkg.com/[email protected]/dist/rapidoc-min.js Sources repo: https://github.com/rapi-doc/RapiDoc

Redoc: https://unpkg.com/[email protected]/bundles/redoc.standalone.js Sources repo: https://github.com/Redocly/ReDoc/

Swagger UI: https://unpkg.com/[email protected]/dist/swagger-ui.js Sources repo: https://github.com/swagger-api/swagger-ui

Where I need to add licenses?

And why we need add licenses only for these libs? Then we should add licenses for each of the libraries we use. But I don't think it makes sense, because micronaut is not a commercial product.

Can you create README.md files in the foldes where these files are. In the contents of the readme.md file point to the original source. Also, mention the license of the libraries in those README.md files. To make sure we are allowed to copy those files into a different open source library and redistributed it.

@altro3
Copy link
Collaborator Author

altro3 commented Nov 8, 2024

Do we need these files in final jar file? Or, maybe I can add it to documentation?

@altro3
Copy link
Collaborator Author

altro3 commented Nov 8, 2024

And still, I don't get it. Why should we add links to licenses of only some open source libraries? Why haven't we added licenses for jackson , freemarker , adoc , etc. What's the difference between javascript scripts and java ?

@sdelamo
Copy link
Contributor

sdelamo commented Nov 12, 2024

And still, I don't get it. Why should we add links to licenses of only some open source libraries? Why haven't we added licenses for jackson , freemarker , adoc , etc. What's the difference between javascript scripts and java ?

I have added README.md files with what I meant.. We need to document where these files come from to ease the update by anyone and we need to document what is the license in the original repository. I want to be sure that we can legally copy them and bundle them in micronaut-openapi. I have also written a bash script that downloads the latest versions. I have run the bash script and updated the JS files.

I would prefer if we did not include minimized js files in our jar. We could just use an absolute link to unpkg.com. That way, it will be less work to maintain and less risk of shipping a security vulnerability in a minimized js file.

@altro3
Copy link
Collaborator Author

altro3 commented Nov 12, 2024

After your changes, will the scripts be included in the final jar file or not?

I will explain why it was done this way: so that when using our library, developers would get a full-fledged solution without external resources. I remember that a couple of years ago there was a request for this and I released such a solution. It would be good if after your changes with downloading scripts, the final archive did not change

@sdelamo
Copy link
Contributor

sdelamo commented Nov 13, 2024

After your changes, will the scripts be included in the final jar file or not?

Yes.

I will explain why it was done this way: so that when using our library, developers would get a full-fledged solution without external resources. I remember that a couple of years ago there was a request for this and I released such a solution.

Yes, I understand the advantage. However, I think the disadvantages are not worth for us. We are now shipping minimised javascript into one of our artifacts. These JS files are hard/impossible to review and may contain vulnerabilities .

It would be good if after your changes with downloading scripts, the final archive did not change.

Sorry, I don't understand what do you mean. I downloaded the js files with the bash script. Do you think it is is incorrect?

@altro3
Copy link
Collaborator Author

altro3 commented Nov 13, 2024

I haven't tested this solution yet, the main thing is that if I run the build locally on Windows, these files are also added to the archive. I often publish to a local repository to test my fixes.

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

Successfully merging this pull request may close these issues.

2 participants