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

Wrote OpenAPI Spec for Self-Defined #72

Closed
wants to merge 2 commits into from

Conversation

hibaymj
Copy link

@hibaymj hibaymj commented Feb 14, 2020

After looking at some of the issues, and taking a look at the site, I made a bunch of guesses at where you would want to take the app with use and such and put together this API Spec to cover those stated and projected goals from the API perspective.

After looking at some of the issues, and taking a look at the site, I made a bunch of guesses at where you would want to take the app with use and such and put together this API Spec to cover those stated and projected goals from the API perspective.
@tatianamac
Copy link
Collaborator

This is so great, @hibaymj ! Thank you!!!

@ovlb would love your thoughts.

@ovlb
Copy link
Collaborator

ovlb commented Feb 16, 2020

@hibaymj FYI: I’ll have a closer look at this in the coming days. Haven’t forgotten it, just a lot on my plate right now.

@tatianamac
Copy link
Collaborator

@ovlb Do you still have a desire to review this? If not, I can see if someone else can!

@ovlb
Copy link
Collaborator

ovlb commented May 12, 2020

@tatianamac As I, finally, have more time at hand, I will catch up with the API discussion and review this PR in the course of the week.

@ovlb
Copy link
Collaborator

ovlb commented May 17, 2020

@hibaymj

Thanks so much for this. I’m no API expert, so I might miss some things. Here are a bunch of questions that sprung to mind:

  • Can we mark some routes as requiring auth/mark them as part of the private API in comparison to the public API (mostly all GET requests)?
  • You have a schema for Word, for NewWord and FullWord could you expand on the differences/use cases for each?
  • Shortlink is a kind of abbreviated definition we could e.g. use to render the overview?

Sorry that it took me ages to get back to you. 😊

@hibaymj
Copy link
Author

hibaymj commented May 21, 2020 via email

word:
$ref: '#/components/schemas/Link'
required:
- shortLink

Choose a reason for hiding this comment

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

Should this be shortLinkText to match the property

Suggested change
- shortLink
- shortLinkText

Comment on lines +62 to +86
Translations:
type: object
properties:
translations:
type: array
minItems: 0
items:
$ref: '#/components/schemas/Link'
Synonyms:
type: object
properties:
synonyms:
type: array
minItems: 0
items:
$ref: '#/components/schemas/Link'
Alternatives:
description: A positive alternative choice to communicate without exclusion.
type: object
properties:
alternatives:
type: array
minItems: 0
items:
$ref: '#/components/schemas/Link'
Copy link

Choose a reason for hiding this comment

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

Should the Translations, Synonyms, and Alternatives be Word types instead of Link types? We can have them in their own containers rather than embedded in the Word object themselves.

type: array
minItems: 0
items:
$ref: '#/components/schemas/Link'
Copy link

Choose a reason for hiding this comment

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

Should the Translations, Synonyms, and Alternatives be Word types instead of Link types? We can have them in their own containers rather than embedded in the Word object themselves.

Suggested change
$ref: '#/components/schemas/Link'
$ref: '#/components/schemas/Word'

type: array
minItems: 0
items:
$ref: '#/components/schemas/Link'

Choose a reason for hiding this comment

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

Suggested change
$ref: '#/components/schemas/Link'
$ref: '#/components/schemas/Word'

type: array
minItems: 0
items:
$ref: '#/components/schemas/Link'

Choose a reason for hiding this comment

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

Suggested change
$ref: '#/components/schemas/Link'
$ref: '#/components/schemas/Word'

Copy link

@aguywithcode aguywithcode left a comment

Choose a reason for hiding this comment

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

I haven't finished going through the entire spec but here are some observations based on what I've seen so far. Would love to hear your thoughts.

originLanguage:
type: string
description: This should be the ISO internationalization value.
tag:

Choose a reason for hiding this comment

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

Tag should be an array because multiple tags might apply. I like the enum but it might be better to allow it to be dynamic and let the API implementation and/or web front end enforce rules around how they're created/added to the word.

@tatianamac tatianamac closed this Jun 5, 2020
@tatianamac
Copy link
Collaborator

@hibaymj Do you mind opening a new PR for this? I changed the repo name and migrated it and it auto-closed every PR. 🙃 Thank you!

@tatianamac
Copy link
Collaborator

@hibaymj Do you mind reopening this as a PR and under the /api repo? Even if you're not able to contribute, I think this start will be invaluable!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
📊 Self-Defined by Category
🍑 Backend/Data/API
Development

Successfully merging this pull request may close these issues.

None yet

4 participants