Methods for Checks API #182
Conversation
|
Thanks! Checks API would be fantastic to have! |
|
@robrbecker I don't think I'll progress on this anytime soon. Also, I have not implemented |
|
Thanks for your work. I'm not in a hurry if you want to take your time. Or maybe you're saying you're not planning on finishing it and you'd like someone to take it over? |
This is what I meant. I don't need the unimplemented methods and I have other priorities for now. |
|
@robrbecker While merging your recent changes into my branch, I had to make the |
|
everything else is public, so I think that'll be fine. |
axel-op
force-pushed
the
checks
branch
3 times, most recently
from
0ee1b21
to
b4d8ed2
Compare
|
I've finally implemented the methods for every endpoint of the Checks API. This PR is ready for review. |
axel-op
force-pushed
the
checks
branch
5 times, most recently
from
342c94a
to
2afd6b7
Compare
lib/src/util.dart
Outdated
| @@ -15,7 +17,8 @@ String buildQueryString(Map<String, dynamic> params) { | |||
| if (params[key] == null) { | |||
| continue; | |||
| } | |||
| queryString.write('$key=${Uri.encodeComponent(params[key].toString())}'); | |||
| // TODO: change to library-specific json encoding | |||
| queryString.write('$key=${Uri.encodeComponent(jsonEncode(params[key]))}'); | |||
There was a problem hiding this comment.
Can you explain why we're jsonEncodeing all objects that are values? This makes me a little nervous that we might break other things so I'd like to understand it better? 🤷
change to library-specific json encoding
Also, I'm not sure I know what you mean by that. Could you give me more information so when I or another person wants to address this TODO we know what we're doing?
There was a problem hiding this comment.
You're right. I made this change following the edit of the type of accepted values in this params map (String to dynamic), but on second thought it's a little risky.
This TODO was related to my other PR, I'll remove it.
My idea was to call an encoding function to serialize values that wouldn't be strings.
| return _urlShortener ??= UrlShortenerService(this); | ||
| } | ||
| UrlShortenerService get urlShortener => | ||
| _urlShortener ??= UrlShortenerService(this); |
There was a problem hiding this comment.
Thanks for shortening up this code and above.
|
@axel-op This looks really good. Just 2 questions really and you'll need to resolve conflicts with master (a method was added in master) |
Add operators to CheckRunAnnotationLevel class
Edit API link for updatePreferencesForCheckSuites and send the request with a body
| other.endLine == this.endLine && | ||
| other.title == this.title && | ||
| other.message == this.message && | ||
| other.rawDetails == this.rawDetails; |
There was a problem hiding this comment.
I'm getting some lints that all these this. are redundant and can be removed
This is a non-breaking PR.
I have implemented some methods to interact with the Checks API (https://developer.github.com/v3/checks) for my personal use, I thought I could make this draft PR to merge them later.