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

Feat/ccrama force sr_detail #5

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ccrama
Copy link

@ccrama ccrama commented Jan 14, 2021

Hello!

sr_detail adds subreddit information including images, descriptions, and user details to any t3 json object returned by the Reddit API.

This PR adds a new SrDetail object to the Submission model. This also forces the sr_detail=1 query parameter on applicable endpoints.

I don't know how you feel about having this as a non-optional parameter, although it doesn't change any behavior from the API except adding additional information to the response.

~ Carlos

@ccrama
Copy link
Author

ccrama commented Jan 14, 2021

Sorry for the .idea changes being included in the PR, I think .idea should potentially be gitignored. I can revert those if you'd like

@KirkBushman
Copy link
Owner

I'll try to give it a look tomorrow.

Copy link
Owner

@KirkBushman KirkBushman left a comment

Choose a reason for hiding this comment

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

I noted some stuff to change, but overall good job.

@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<project version="4">
<component name="CompilerConfiguration">
<bytecodeTargetLevel target="14" />
<bytecodeTargetLevel target="1.8" />
Copy link
Owner

Choose a reason for hiding this comment

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

As you said I should remove the .idea folder, I used it mainly to sync between computers. I'm trying to add some features at the moment, after that's done, I'll try to clean up stuff. Both in code and in project.

In the meantime I would revert this, please.

@@ -0,0 +1,10 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Owner

Choose a reason for hiding this comment

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

Revert this as well, please.

@@ -0,0 +1,46 @@
<?xml version="1.0" encoding="UTF-8"?>
<project version="4">
Copy link
Owner

Choose a reason for hiding this comment

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

Revert this as well, please.

@@ -5,7 +5,7 @@
<configuration PROFILE_NAME="Debug" CONFIG_NAME="Debug" />
</configurations>
</component>
<component name="ProjectRootManager" version="2" languageLevel="JDK_14_PREVIEW" default="false" project-jdk-name="11" project-jdk-type="JavaSDK">
<component name="ProjectRootManager" version="2" languageLevel="JDK_1_8" default="false" project-jdk-name="1.8" project-jdk-type="JavaSDK">
Copy link
Owner

Choose a reason for hiding this comment

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

Revert this as well, please.

@@ -297,6 +298,7 @@ interface RedditApi {
@Query("after") after: String? = null,
@Query("before") before: String? = null,
@Query("raw_json") rawJson: Int? = null,
@Query("sr_detail") srDetail: Int = 1,
Copy link
Owner

Choose a reason for hiding this comment

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

Make the type nullable and the default null, please.

@@ -654,6 +665,7 @@ interface RedditApi {
@Query("after") after: String? = null,
@Query("before") before: String? = null,
@Query("raw_json") rawJson: Int? = null,
@Query("sr_detail") srDetail: Int = 1,
Copy link
Owner

Choose a reason for hiding this comment

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

Make the type nullable and the default null, please.

@@ -669,6 +681,7 @@ interface RedditApi {
@Query("after") after: String? = null,
@Query("before") before: String? = null,
@Query("raw_json") rawJson: Int? = null,
@Query("sr_detail") srDetail: Int = 1,
Copy link
Owner

Choose a reason for hiding this comment

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

Make the type nullable and the default null, please.

@@ -684,6 +697,7 @@ interface RedditApi {
@Query("after") after: String? = null,
@Query("before") before: String? = null,
@Query("raw_json") rawJson: Int? = null,
@Query("sr_detail") srDetail: Int = 1,
Copy link
Owner

Choose a reason for hiding this comment

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

Make the type nullable and the default null, please.

val publicDescription: String?,

@Json(name = "user_is_subscriber")
val userIsSubscriber: Boolean?
Copy link
Owner

Choose a reason for hiding this comment

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

Convert the naming into isSubscriber, isMuted like the rest, please

@@ -142,6 +142,9 @@ data class Submission(
@Json(name = "gildings")
override val gildings: Gildings,

@Json(name = "sr_detail")
val sr_detail: SrDetail?,
Copy link
Owner

Choose a reason for hiding this comment

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

Use camel case for the variable name here, otherwise detekt will error the build in Travis. Move it to srDetail, please.

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.

2 participants