GraphQL - Embeds by toyhammered · Pull Request #898 · hummingbird-me/kitsu-server

toyhammered · 2020-11-27T21:47:33Z

What

Why

Checklist

All files pass Rubocop
Any complex logic is commented
Any new systems have thorough documentation
Any user-facing changes are behind a feature flag (or: explain why they can't be)

All the tests pass
Tests have been added to cover the new code

hound · 2020-11-27T21:47:39Z

+  field :site_name, String,
+    description: SITE_NAME_DESCRIPTION,
+    null: false
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

NuckChorris · 2020-11-28T05:15:39Z

@@ -0,0 +1,5 @@
+class Types::Union::EmbedItem < Types::Union::Base


Is there a good argument to using a union vs just referencing the base embed interface that all embeds inherit from?

I'm not actually sure. I've always preferred using a union vs referencing the base embed, I think it reads nicer. But both function the same way.

After reading this: https://artsy.github.io/blog/2019/01/14/graphql-union-vs-interface/ I think I prefer Unions.

hound · 2020-11-28T19:37:50Z

+
+  field :url, String,
+    null: false,
+    description: 'The canonical URL of your object that will be used as its permanent ID in the graph'


Line is too long. [102/100] (https://rubystyle.guide#80-character-limits)

hound · 2020-11-28T19:37:50Z

+  value 'MALE', value: 'male'
+  value 'FEMALE', value: 'female'
+  value 'OTHER', value: 'other'
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

hound · 2020-11-28T19:37:50Z

+  field :gender, Types::Enum::Gender,
+    null: true,
+    description: 'Their gender.'
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

hound · 2020-11-28T19:37:50Z

+  implements Types::Interface::BaseEmbed
+
+  description 'Audio Properties'
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

NuckChorris · 2020-11-28T19:50:10Z

+  field :determiner, String,
+    null: true,
+    description: <<~DESCRIPTION.squish
+      The word that appears before this object's title in a sentence.
+      An enum of (a, an, the, "", auto). If auto is chosen,
+      the consumer of your data should chose between "a" or "an". Default is "" (blank).
+    DESCRIPTION
+
+  field :locale, String,
+    null: true,
+    description: <<~DESCRIPTION.squish
+      The locale these tags are marked up in.
+      Of the format language_territory.
+      Default is en_us.
+    DESCRIPTION
+
+  field :locale_alternative, [String],
+    null: true,
+    description: 'An array of other locales this page is available in.'


I don't think we use any of these

should we not start bringing them in? They are optional.

NuckChorris · 2020-11-28T19:50:29Z

@@ -0,0 +1,5 @@
+class Types::Embed::AudioTagEmbed < Types::BaseObject


Why the Tag?

seemed kind of strange just being AudioEmbed and on the docs it used tag in the description. i.e: The og:video tag has the identical tags which I kinda liked. I can rename though.

hound · 2020-11-29T18:21:32Z

+class Types::Embed::VideoEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

hound · 2020-11-29T18:21:32Z

@@ -0,0 +1,4 @@
+class Types::Embed::VideoEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+


Extra empty line detected at class body end. (https://rubystyle.guide#empty-lines-around-bodies)

hound · 2020-11-29T18:21:32Z

+class Types::Embed::MusicEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

hound · 2020-11-29T18:21:32Z

@@ -0,0 +1,4 @@
+class Types::Embed::MusicEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+


Extra empty line detected at class body end. (https://rubystyle.guide#empty-lines-around-bodies)

hound · 2020-11-29T18:21:33Z

@@ -0,0 +1,3 @@
+class Types::Embed::ImageEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

hound · 2020-11-29T18:21:33Z

@@ -0,0 +1,3 @@
+class Types::Embed::BookEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

hound · 2020-11-29T18:21:33Z

@@ -0,0 +1,3 @@
+class Types::Embed::WebsiteEmbed < Types::BaseObject
+  implements Types::Interface::BaseEmbed
+end


Final newline missing. (https://rubystyle.guide#newline-eof)

qlty-cloud-legacy · 2020-11-29T18:22:24Z

Code Climate has analyzed commit 5e9c522 and detected 12 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Style	12

View more on Code Climate.

base setup for post embeds

34e0d65

toyhammered added the WIP label Nov 27, 2020

toyhammered self-assigned this Nov 27, 2020

hound Bot reviewed Nov 27, 2020

View reviewed changes

NuckChorris requested changes Nov 28, 2020

View reviewed changes

toyhammered added 4 commits November 28, 2020 11:15

change to use base embed as interface

a04db9d

base tag embeds

9424ba9

website embed

913b1c4

profile embed

7d4a713

hound Bot reviewed Nov 28, 2020

View reviewed changes

toyhammered added 3 commits November 28, 2020 11:41

fix base embed tag pathings

cc45c46

remove embed union in favor of interface

9fb779b

implement correct embed interface

65ef417

toyhammered requested a review from NuckChorris November 28, 2020 19:45

NuckChorris reviewed Nov 28, 2020

View reviewed changes

toyhammered added 2 commits November 28, 2020 13:30

more renaming stuff

ef6c4c9

additional embeds

5e9c522

hound Bot reviewed Nov 29, 2020

View reviewed changes

NuckChorris added this to the V4: Feeds milestone Dec 5, 2021

		@@ -0,0 +1,5 @@
		class Types::Union::EmbedItem < Types::Union::Base

		@@ -0,0 +1,5 @@
		class Types::Embed::AudioTagEmbed < Types::BaseObject

		@@ -0,0 +1,4 @@
		class Types::Embed::VideoEmbed < Types::BaseObject
		implements Types::Interface::BaseEmbed

		@@ -0,0 +1,4 @@
		class Types::Embed::MusicEmbed < Types::BaseObject
		implements Types::Interface::BaseEmbed

Uh oh!

Conversation

toyhammered commented Nov 27, 2020

What

Why

Checklist

Uh oh!

hound Bot Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

NuckChorris Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

toyhammered Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

toyhammered Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

NuckChorris Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

toyhammered Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

NuckChorris Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

toyhammered Nov 28, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

hound Bot Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

qlty-cloud-legacy Bot commented Nov 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants