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

Spec Insert #8472

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

nhtruong
Copy link
Contributor

@nhtruong nhtruong commented Oct 7, 2024

Description

A Program to insert API components generated from OpenSearch Spec into markdown files.
Check out the included README.md file for more details.

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

github-actions bot commented Oct 7, 2024

Thank you for submitting your PR. The PR states are In progress (or Draft) -> Tech review -> Doc review -> Editorial review -> Merged.

Before you submit your PR for doc review, make sure the content is technically accurate. If you need help finding a tech reviewer, tag a maintainer.

When you're ready for doc review, tag the assignee of this PR. The doc reviewer may push edits to the PR directly or leave comments and editorial suggestions for you to address (let us know in a comment if you have a preference). The doc reviewer will arrange for an editorial review.

A program that insert API Components generated from the OpenSearch OpenAPI Spec into markdown files

Signed-off-by: Theo Truong <[email protected]>
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Code looks good. I made some nitpicks below, but at a higher level I think we should debate what form this takes.

I think it's important to not merge this without working CI/CD. I would say this is my one must have.

I think the tool should not be a command line processor nor a rake task. This project is already a Ruby project based on Jekyll that supports plugins. It would be better as a plugin because doc writers would run jekyll serve locally, and keep updating the docs, while Jekyll re-generates the pages that have changed in the background, no need to constantly run the tool command line. In an editor you'd see changes almost immediately get reloaded and can keep editing the document. Otherwise authors of the documentation have to know the special process of invoking the command and keep invoking it all the time. It's easy to disable the plugin when deploying the site too, and it could emit a warning that content may have changed. Finally, the plugin could (re)render only the pages that have changed, which is a lot faster.

I don't love the name. I think the tool will do more than use the spec, potentially applying other transformations like maybe running examples. Another idea for a name could be render.

@@ -0,0 +1,25 @@
require: rubocop-rake
AllCops:
Include:
Copy link
Member

Choose a reason for hiding this comment

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

Should apply to specs to, use rubocop -a ; rubocop --auto-gen-config in general.

@@ -0,0 +1,91 @@
# README: Spec Insert
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# README: Spec Insert
# Spec Insert

- [Ignored files and folders](#ignored-files-and-folders)

## What is this?
This program allows you to insert API components generated from the OpenSearch Specification into this repository's markdown files. It's still underdevelopment, and many features are not yet implemented. This document will be updated as the program evolves.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This program allows you to insert API components generated from the OpenSearch Specification into this repository's markdown files. It's still underdevelopment, and many features are not yet implemented. This document will be updated as the program evolves.
The spec_insert tool allows you to insert API components generated from the [OpenSearch Specification](https://github.com/opensearch-project/opensearch-api-specification) into this repository's markdown files. It's still underdevelopment, and many features are not yet implemented. This document will be updated as the program evolves.

3. Install Ruby 3.1.0 or later.
4. Install the required gems by running `bundle install`.

## How to use
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## How to use
## How to Use

```

### Ignored files and folders
The program will ignore all markdown files whose names are in ALL CAPS. On top of that, you can also add files and folders you want to the [ignored.txt](./ignored.txt) file. Each line in the file should be the name of a file or folder you want to ignore.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should name ignored.txt as .spec_insert_ignore to be consistent with gitignore and friends.

'https://github.com/opensearch-project/opensearch-api-specification' \
'/releases/download/main-latest/opensearch-openapi.yaml ' \
'-o opensearch-openapi.yaml'
end
Copy link
Member

Choose a reason for hiding this comment

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

Download to a temp or _site folder to avoid git add-ing it later.

Don't use sh curl, it will be harder to debug, may not work on Windows, etc.

openapi_url = "https://github.com/opensearch-project/opensearch-api-specification/releases/download/main-latest/opensearch-openapi.yaml"
openapi_path = "_site/opensearch-openapi.yaml"
File.write(openapi_path, URI.open(openapi_url).read)

root_folder: '../',
spec_file: './opensearch-openapi.yaml',
ignored: './ignored.txt'
).insert_spec
Copy link
Member

Choose a reason for hiding this comment

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

rename the method to have a ! ruby-style, so insert!

@nhtruong
Copy link
Contributor Author

nhtruong commented Oct 8, 2024

Thanks @dblock
Converting this to draft until I can get back to this issue on a later date.

@nhtruong nhtruong marked this pull request as draft October 8, 2024 15:28
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.

4 participants