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

Remove examples folder [main branch] #3913

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Bellangelo
Copy link
Member

@Bellangelo Bellangelo commented Jun 15, 2024

Partially closes #3854

We have moved in this OAI/learn.openapis.org#102 the examples to the learn.openapis.org repo. So in this PR we are cleaning the main branch by removing the examples folder and making any related changes.

v3.0.4-dev: #3914
v3.1.1-dev: #3915
v3.2.0-dev: #3916

@Bellangelo Bellangelo requested review from a team as code owners June 15, 2024 00:07
@handrews handrews added the examples requests for more or better examples in the specification label Jun 15, 2024
@@ -1821,7 +1821,7 @@ The key value used to identify the callback object is an expression, evaluated a
##### Patterned Fields
Field Pattern | Type | Description
---|:---:|---
<a name="callbackExpression"></a>{expression} | [Path Item Object](#path-item-object) | A Path Item Object used to define a callback request and expected responses. A [complete example](../examples/v3.0/callback-example.yaml) is available.
<a name="callbackExpression"></a>{expression} | [Path Item Object](#path-item-object) | A Path Item Object used to define a callback request and expected responses. A [complete example](https://github.com/OAI/learn.openapis.org/tree/main/examples/v3.0/callback-example.yaml) is available.
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to link to wherever we host these examples on the actual learn.openapis.org site. One thing we're trying to do by moving these examples out of this repo is to stop linking into GitHub repos and only treat spec.openapis.org and learn.openapis.org as official published locations. This might require getting them published on the learn site first if they're not on a page and are instead just sitting there right now.

Copy link
Member Author

@Bellangelo Bellangelo Jun 15, 2024

Choose a reason for hiding this comment

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

@handrews Something like this link https://learn.openapis.org/examples/v3.1/tictactoe.yaml? If yes, then it seems that we are lucky since the whole folder is already deployed.
Although I have one question, I understand why we want to point to the website but are we sure that this is currently the best thing we should be doing? I am asking because Github formats the file better than the browser itself.

Copy link
Member

Choose a reason for hiding this comment

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

Hm... we should add something to the Learn site's build to get them to render as syntax highlighted HTML. I'd rather take the extra time to do that than to move to an interim place, have people get used to that, and then have to convince them to stop looking at the interim place. Particularly because we obviously need to have these in GitHub somewhere. I just think folks should look at the Learn and Spec sites and ideally stop looking at GitHub unless they need to do work in the repository (including filing issues, commenting, etc.).

But I am totally making this plan up as I type it, and others might feel differently.

Copy link
Member

Choose a reason for hiding this comment

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

With a bit of experimentation, it looks like we could move the JSON and YAML files under _includes and do something like this for each one:

---
layout: default
title: Tic-Tac-Toe Example (YAML)
parent: Examples
nav_order: 1
has_toc: false 
---

{% highlight yaml %}
{% include examples/v3.1/tictactoe.yaml %}
{% endhighlight %}

ralfhandl
ralfhandl previously approved these changes Jun 17, 2024
@ralfhandl ralfhandl requested review from handrews and a team June 17, 2024 13:47
@ralfhandl ralfhandl dismissed their stale review June 18, 2024 09:48

Wait for further changes

Copy link
Member

@handrews handrews left a comment

Choose a reason for hiding this comment

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

@Bellangelo we discussed this in the TDC call this morning and agreed that we need the links to go to rendered pages on the Learn site. So please, when you get a chance, if you could add pages there first, and once that is approved we can finish these PRs out here. LMK if you encounter any problems on the Learn site PRs and I'm happy to help.

@mikekistler
Copy link
Contributor

@ralfhandl Did you incorporate these changes into one of your recent PRs? This looks familiar. If yes, can we close this one?

@ralfhandl
Copy link
Contributor

ralfhandl commented Aug 20, 2024

Did you incorporate these changes into one of your recent PRs?

No, the examples were copied from the OAS repo to the learn repo via OAI/learn.openapis.org#102, and this cleanup is still pending.

This PR needs to be adjusted to reflect @handrews comment, which needs OAI/learn.openapis.org#105 to be merged first.

And we need two more PRs to adjust 3.0.4.md and 3.1.1.md in the corresponding dev branches:

@ralfhandl ralfhandl marked this pull request as draft August 23, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples requests for more or better examples in the specification
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

Move examples to learn.openapis.org or spec.openapis.org
4 participants