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

ci: Update GH API #1378

Merged
merged 4 commits into from
Dec 9, 2020
Merged

ci: Update GH API #1378

merged 4 commits into from
Dec 9, 2020

Conversation

pawelpasterz
Copy link
Contributor

Fixes #1377

This PR adds new requests for flank's GH client. Currently not used. Commit was cherry-picked to make future PR (related to flank scripts/IT) smaller

Test Plan

How do we know the code works?

N/A

@pawelpasterz pawelpasterz self-assigned this Dec 7, 2020
@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2020

Timestamp: 2020-12-08 13:38:02
Buildscan url for ubuntu-workflow run 408321670
https://gradle.com/s/gn2a3cpzxr3qs


@Serializable
enum class IssueState {
@SerialName("open") OPEN,
Copy link
Contributor

Choose a reason for hiding this comment

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

I want to confirm if the kotlinx serialization is fully compatible with the GSON we are making use of
Else if might result in us changing all the annotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they are, not 100% sure though. In flank-scripts module we do not use GSON.

Copy link
Contributor

Choose a reason for hiding this comment

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

I saw here we are DE serializing with GSON so just want to confirm.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wait what? We should not do that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Sloox could you give an example? I am unable to find

@bootstraponline bootstraponline force-pushed the 1377-update-github-API branch 3 times, most recently from 1e288ee to b4d5e22 Compare December 7, 2020 18:00
@codecov-io
Copy link

Codecov Report

Merging #1378 (b4d5e22) into master (5081a69) will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1378      +/-   ##
============================================
- Coverage     77.84%   77.82%   -0.03%     
  Complexity      706      706              
============================================
  Files           245      245              
  Lines          4713     4713              
  Branches        904      904              
============================================
- Hits           3669     3668       -1     
  Misses          551      551              
- Partials        493      494       +1     

Copy link
Contributor

@Sloox Sloox left a comment

Choose a reason for hiding this comment

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

Had a look for the Serialization compatibility. It appears to be fine as far as I am aware.

@mergify mergify bot merged commit 2d29824 into master Dec 9, 2020
@mergify mergify bot deleted the 1377-update-github-API branch December 9, 2020 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[IT] Add requests to GithubApi.kt
5 participants