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
fix: fix issue #1325 and add missing truncated
field in the GHGist
object
#1339
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing.
Sorry about all the comments. This is basically a good PR with test coverage an everything, but you ran it in a bunch of the gotchas of working in the library all at once.
I'm happy to answer any questions you might have about the comments.
2bd488b
to
655323b
Compare
@bitwiseman. Thank you so much for your help and detailed comment, bitwiseman! All of those suggestions are super helpful and I have made revisions based on your suggestions! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few more bits of feedback.
public void setOwner(GHUser owner) { | ||
this.owner = owner; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, why add this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, why add this?
Sorry, the reason I am adding this setter is that I want to pass the owner
object from the GHGist
object to each GHGistFile
object so that get content can use owner.root()
to create request. At a second thought, probably defining a wrapUp
method to set the owner as the GHGist
object owning this current GHGistFile
object would make more sense. I have make this revision accordingly. Thank you again for your suggestions.
655323b
to
53c0978
Compare
53c0978
to
2aeda8e
Compare
truncate
field in the GHGist
objecttruncated
field in the GHGist
object
@SCHJonathan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like some test is failing.
Description
raw_url
field ofGHGistFile
object if the gist filecontent
is uninitialized.truncated
field in theGHGist
object and I added that field in this PR.Before submitting a PR:
mvn -D enable-ci clean install site
locally. If this command doesn't succeed, your change will not pass CI.main
. You will create your PR from that branch.When creating a PR: