added fields in GHPullRequestReviewComment which were missing #1469

kisaga · 2022-06-03T16:49:11Z

Description

added new fields in GHPullRequestReviewComment that were missing.
Assuming that the changes are minor the mocked data snapshot has not been updated.

The response and its JSON Schema can be found here
fixes #1463

Before submitting a PR:

Changes must not break binary backwards compatibility. If you are unclear on how to make the change you think is needed while maintaining backward compatibility, CONTRIBUTING.md for details.
Add JavaDocs and other comments as appropriate. Consider including links in comments to relevant documentation on https://docs.github.com/en/rest .
Add tests that cover any added or changed code. This generally requires capturing snapshot test data. See CONTRIBUTING.md for details.
Run mvn -D enable-ci clean install site locally. If this command doesn't succeed, your change will not pass CI.
Push your changes to a branch other than main. You will create your PR from that branch.

When creating a PR:

Fill in the "Description" above with clear summary of the changes. This includes:
If this PR fixes one or more issues, include "Fixes #" lines for each issue.
Provide links to relevant documentation on https://docs.github.com/en/rest where possible.
All lines of new code should be covered by tests as reported by code coverage. Any lines that are not covered must have PR comments explaining why they cannot be covered. For example, "Reaching this particular exception is hard and is not a particular common scenario."
Enable "Allow edits from maintainers".

gsmet · 2022-06-16T12:31:24Z

Assuming that the changes are minor the mocked data snapshot has not been updated.

I think this is gonna be a problem. We need to be able to regenerate the mocked data and still have the tests passing. You should probably create a test project in the test org, and add a review there that allows to test this thing.

gsmet · 2022-06-16T12:31:25Z

Assuming that the changes are minor the mocked data snapshot has not been updated.

I think this is gonna be a problem. We need to be able to regenerate the mocked data and still have the tests passing. You should probably create a test project in the test org, and add a review there that allows to test this thing.

kisaga · 2022-06-16T21:12:54Z

Cool! I'll check how to create a project in the test org and how to update the mock data.

kisaga · 2022-06-17T17:10:40Z

Hi @gsmet I've run the tests against the https://github.com/hub4j-test-org/github-api twice:

And I have some questions:

In the Contributing guide it is mentioned Please request access to this org to record your tests before submitting a PR.. From whom should I request access ? The two tests I've run are failing at the validation of AuthorAssociation field because mine is NONE but the expected is MEMBER.
Is it a problem if I continue using this project to modify the setup phase of the test I've modified ? (I need to add some reactions to get the respective field in the response)
For the final snapshot that will be committed what should be the target repository for the responses ?

Waiting for your feedback 🙂

kisaga · 2022-06-19T20:37:19Z

Hi @gsmet,
I managed to take a snapshot after modifying the test to add some reactions, fetch again the comment and check the reactions. I've also modified some values that I had hardcoded in the mock responses in my initial approach.
The snapshot is from this PR and I have modified manually the author_association from NONE to MEMBER.
I don't know if the snapshot is fine or if it should be taken as a MEMBER, but even if this the case, if I become member of the test org I can take a new snapshot and commit the changes.

Let me know if it is fine or if I should take the snapshot as a MEMBER (if yes, how can I join the test org).

bitwiseman · 2022-06-20T20:35:03Z

@kisaga
I've invited you to the test org. You should be able to take Snapshots as a member now.

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

bitwiseman · 2022-06-20T20:54:06Z

src/main/java/org/kohsuke/github/GHPullRequestReviewCommentReactions.java

+ * @author Vasilis Gakias
+ * @see <a href="https://docs.github.com/en/rest/pulls/comments#get-a-review-comment-for-a-pull-request">API
+ *      documentation</a>
+ * @see GHPullRequestReviewComment
+ */
+public class GHPullRequestReviewCommentReactions {


I am not seeing the reactions field listed in the doc link mentioned here.
This page talks about reactions API:
https://docs.github.com/en/rest/reactions#list-reactions-for-a-pull-request-review-comment

Apologies. Updated with the "get comments for a pull request" link

bitwiseman

Please double check that the reactions field is actually part of the supported API and not deprecated and soon to be removed.

codecov · 2022-06-20T21:04:35Z

Codecov Report

Base: 79.56% // Head: 79.63% // Increases project coverage by +0.07% 🎉

Coverage data is based on head (83324ca) compared to base (24832b1).
Patch coverage: 92.30% of modified lines in pull request are covered.

❗ Current head 83324ca differs from pull request most recent head 248e4f2. Consider uploading reports for the commit 248e4f2 to get more accurate results

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1469      +/-   ##
============================================
+ Coverage     79.56%   79.63%   +0.07%     
- Complexity     2166     2187      +21     
============================================
  Files           207      208       +1     
  Lines          6616     6655      +39     
  Branches        364      364              
============================================
+ Hits           5264     5300      +36     
- Misses         1140     1141       +1     
- Partials        212      214       +2

Impacted Files	Coverage Δ
...org/kohsuke/github/GHPullRequestReviewComment.java	`88.00% <84.21%> (-1.29%)`	⬇️
...ke/github/GHPullRequestReviewCommentReactions.java	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

modified test, took snapshot and modified manually the author_association from NONE to MEMBER

kisaga

Avoiding `reactions` field deprecation

I've checked here and I couldn't see any deprecation notice regarding the reactions field.
I've also queried twitter for any reaction related changes but I didn't find anything related except for this which is not related to these changes.

Data snapshot as a member

I've also updated the snapshot with a membership and the files are fine now.

Pending

Let me know if we want to change the pull_request_review_id from Long to long with the side effect of having 0 default value instead of -1 if the value the response is null. I've found this but I couldn't apply it and also could not find similar in this project

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

kisaga · 2022-06-21T17:37:03Z

src/main/java/org/kohsuke/github/GHPullRequestReviewCommentReactions.java

+ * @author Vasilis Gakias
+ * @see <a href="https://docs.github.com/en/rest/pulls/comments#get-a-review-comment-for-a-pull-request">API
+ *      documentation</a>
+ * @see GHPullRequestReviewComment
+ */
+public class GHPullRequestReviewCommentReactions {


Apologies. Updated with the "get comments for a pull request" link

bitwiseman · 2022-06-22T03:29:01Z

💢 So, the reactions field shows up in the "response schema" on that section but not in the "example response". :(

That page gives me no indication of the right path forward. :( Not your fault. I'm concerned because the "Reaction API" paged don't talk about this field. Is it because it is new and they haven't documented it, or because it is old and will eventually be replaced? I do not know.

https://docs.github.com/en/rest/pulls/comments#get-a-review-comment-for-a-pull-request

...
    "reactions": {
      "title": "Reaction Rollup",
      "type": "object",
      "properties": {
        "url": {
          "type": "string",
          "format": "uri"
        },
        "total_count": {
          "type": "integer"
        },
        "+1": {
          "type": "integer"
        },
        "-1": {
          "type": "integer"
        },
        "laugh": {
          "type": "integer"
        },
        "confused": {
          "type": "integer"
        },
        "heart": {
          "type": "integer"
        },
        "hooray": {
          "type": "integer"
        },
        "eyes": {
          "type": "integer"
        },
        "rocket": {
          "type": "integer"
        }
      },
      "required": [
        "url",
        "total_count",
        "+1",
        "-1",
        "laugh",
        "confused",
        "heart",
        "hooray",
        "eyes",
        "rocket"
      ]
    },
...

kisaga · 2022-06-22T15:25:34Z

HI @bitwiseman I've asked a question here.
We can wait for an answer and then make our decision for this field.
If we need the rest of the changes we can:

either include the reactions field and remove it if the answer will be that it'll removed at some point. In this case if it will be deprecated it will have a null value
or exclude it and add it in case the answer is that it will not be removed

bitwiseman · 2022-06-22T18:03:01Z

Please exclude it until we have an answer. It might make sense to start a new PR for the other changes, since they shouldn't even require new test data, just some additional asserts.

bitwiseman · 2022-08-18T17:31:58Z

@kisaga Please make the change requested above so we can merge the rest of this PR.

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

kisaga changed the title ~~added fields in GHPullRequestReviewComment related which were missing~~ added fields in GHPullRequestReviewComment which were missing Jun 3, 2022

kisaga force-pushed the issue/enhance-pull-request-comment-model branch from af42287 to f43de91 Compare June 19, 2022 20:27

bitwiseman reviewed Jun 20, 2022

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java Show resolved Hide resolved

bitwiseman reviewed Jun 20, 2022

View reviewed changes

bitwiseman requested changes Jun 20, 2022

View reviewed changes

added fields in GHPullRequestReviewComment related which were missing

8494720

modified test, took snapshot and modified manually the author_association from NONE to MEMBER

kisaga force-pushed the issue/enhance-pull-request-comment-model branch from f43de91 to 8494720 Compare June 21, 2022 17:22

kisaga commented Jun 21, 2022

View reviewed changes

bitwiseman marked this pull request as draft August 18, 2022 17:35

bitwiseman added the work-abandoned There hasn't been any activity on the PR in a while. Another contributor might want to pick it up. label Nov 12, 2022

bitwiseman marked this pull request as ready for review February 26, 2023 19:55

Merge branch 'main' into issue/enhance-pull-request-comment-model

0b185f2

bitwiseman reviewed Feb 26, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java Show resolved Hide resolved

Update src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

3b72141

bitwiseman reviewed Feb 26, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java Outdated Show resolved Hide resolved

Update src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

83324ca

bitwiseman reviewed Feb 26, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java Outdated Show resolved Hide resolved

Update src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

35dcc05

bitwiseman reviewed Feb 26, 2023

View reviewed changes

src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java Outdated Show resolved Hide resolved

Update src/main/java/org/kohsuke/github/GHPullRequestReviewComment.java

248e4f2

bitwiseman self-requested a review February 26, 2023 20:27

bitwiseman merged commit 49d8ed2 into hub4j:main Feb 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added fields in GHPullRequestReviewComment which were missing #1469

added fields in GHPullRequestReviewComment which were missing #1469

kisaga commented Jun 3, 2022 •

edited

Loading

gsmet commented Jun 16, 2022

gsmet commented Jun 16, 2022

kisaga commented Jun 16, 2022

kisaga commented Jun 17, 2022 •

edited

Loading

kisaga commented Jun 19, 2022

bitwiseman commented Jun 20, 2022 •

edited

Loading

bitwiseman Jun 20, 2022

kisaga Jun 21, 2022

bitwiseman left a comment

codecov bot commented Jun 20, 2022 •

edited

Loading

kisaga left a comment

kisaga Jun 21, 2022

bitwiseman commented Jun 22, 2022 •

edited

Loading

kisaga commented Jun 22, 2022

bitwiseman commented Jun 22, 2022

bitwiseman commented Aug 18, 2022

added fields in GHPullRequestReviewComment which were missing #1469

added fields in GHPullRequestReviewComment which were missing #1469

Conversation

kisaga commented Jun 3, 2022 • edited Loading

Description

Before submitting a PR:

When creating a PR:

gsmet commented Jun 16, 2022

gsmet commented Jun 16, 2022

kisaga commented Jun 16, 2022

kisaga commented Jun 17, 2022 • edited Loading

kisaga commented Jun 19, 2022

bitwiseman commented Jun 20, 2022 • edited Loading

bitwiseman Jun 20, 2022

Choose a reason for hiding this comment

kisaga Jun 21, 2022

Choose a reason for hiding this comment

bitwiseman left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 20, 2022 • edited Loading

Codecov Report

kisaga left a comment

Choose a reason for hiding this comment

Avoiding reactions field deprecation

Data snapshot as a member

Pending

kisaga Jun 21, 2022

Choose a reason for hiding this comment

bitwiseman commented Jun 22, 2022 • edited Loading

kisaga commented Jun 22, 2022

bitwiseman commented Jun 22, 2022

bitwiseman commented Aug 18, 2022

kisaga commented Jun 3, 2022 •

edited

Loading

kisaga commented Jun 17, 2022 •

edited

Loading

bitwiseman commented Jun 20, 2022 •

edited

Loading

codecov bot commented Jun 20, 2022 •

edited

Loading

Avoiding `reactions` field deprecation

bitwiseman commented Jun 22, 2022 •

edited

Loading